On 25.04.2013 12:34, Matěj Laitl wrote:
Oh, nice! Do mention it. I may be stating the obvious, but I felt it
needs to be said: Please note that "ideal" code for a typical FLOSS
project may be very different from "ideal" code for a programming
contest. In Amarok, we try to keep code as simple (to read &
understand) as possible, even if it would mean it is let's say 15%
slower. Keep in mind that many readers of our code may not be contest
finalists - but we still want them to be able to understand and
perhaps modify it. This of course doesn't mean you should be using
O(n) algorithm when O(log(n)) algorithm exists or refrain from using
private inheritance where it is appropriate. It means "oh, this is a
super-smart code, but perhaps it can be expressed more clearly using a
line or two more?" mindset.
Now I feel that I need to respond to that. :-)
I'm a mature programmer. I have well over five years of real programming
experience (not the home-grown kind), and in that there's over half a
year of commercial experience. I'm not claiming I'm very experienced -
obviously five years is a lot less than ten or fifteen, but still that's
enough to have both "fastest code" and "shortest code" phases well
behind me. I favor clean, easily understandable and maintanable code,
making (sensible) sacrifices of speed and linecount. I've also
programmed in assembly a bit and analyzed how compilers work, so I'm
very much aware that modern compilers more often than not optimize the
code that makes its intentions clear to be actually faster than
hand-optimized code doing the same thing.
That's basically what I've done in my first (of many, hopefully) review;
the cleanup wasn't to make it faster, it was for it to be understandable
(e.g. I myself had problems with figuring out what value is actually
returned), and therefore of better quality.
And to touch on the private inheritance. ;-) Although it's seldom used,
it actually makes code clearer and gives out more information at a
glance than composition. (Of course composition should be favored over
inheritance, but inheritance should be used where appropriate.) The
thing is to read it the right way: while you'd read "A: public
BaseClass" as "A is a BaseClass", private inheritance would be read as
"A is implemented in terms of BaseClass". So now that very single-line
declaration gives you information that underneath A is built on top of
BaseClass - and all of that without looking through A's actual
implementation to see that A is a BaseClass wrapper. It's all about
semantics and choosing a language construct that best represents what
the code is supposed to do.
Matěj
Konrad
_______________________________________________
Amarok-devel mailing list
Amarok-devel@kde.org
https://mail.kde.org/mailman/listinfo/amarok-devel
