Dear Peter,

I totally agree with you about the assert….

About copyBytes, I would leave it in… as it is more readable and one very good rule to follow is being cohesive… One function should do only one thing etc.. )
I remember skimming thru PJP’s “Standard C Library”. The code was full of small functions … and so lovely and readable ;-) Another book that comes to mind is “Compiler design in C” by Allen Holub — the same rule applies to good  C++ ( “Practice Of Programming”, EC++, etc. )
About inlining, however, I would rather not do it… See Herb Sutter’s “More Exceptional C++” Item 12 ( inline ). The gist is to inline only when a profiler tells me to do it. And the compiler knows better and will inline wherever needed ( even C compilers do inlining, AFAIK, even though traditionally C does not have inline — C99 I think has ;-) - and virtual functions can also be inlined… etc…

So the net net is not to worry about it ( until it is a bottleneck as told by a profiler ). I mean, we should be idiomatic ( always using ++iter instead of iter++ in a for loop, for example ;-) and follow usual optimization idioms… and look at algorithms / data structures first etc.. )
Let me know what you all think…

—- cheerio atul

Peter Dimov wrote:

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