Hi Atul,

Thanks for the patch! The functions look so much simpler now ;-)! I allowed myself to make the new functions inline and thus give the compiler a clue where optimizations could be performed. I also removed yet another duplication in canRead() and documented the contract (precondition) the caller of getRangePtrs() should fulfill. I think it is better to assert than simply return 0. This way we will easily locate the place the precondition was violated – that’s what design by contract advocates.

copyBytes() is now only called by doRead() and it’s quite simple… does it really deserve to be a function of its own?

Attached is a ready for commit patch (based on revno 76) of the combined changes. Should I commit or somebody would have further refactoring ideas ;-)?

I think that similar refactoring can be applied to BinaryWriter as well.

Regards,

Peter

atul wrote:Hey all

This is one of my favourite refactorings ;-) If you can get “Working with Legacy Code” by Feathers, there is a fascinating chapter in which he just removes duplication and the design just comes out … It was one of the Aha! moments for me ;-)
Here is the patch …. All the tests pass, btw ;-)

— cheerio atul