Re: Uncommented code in LilyPond

[address@hidden]

On 3 sept. 2012, at 11:45, David Kastrup <address@hidden> wrote:

> "address@hidden" <address@hidden> writes:
> 
>> As a frequent producer of uncommented code that pushes into a code
>> base with uncommented code, I am more than willing to put comments.
>> However, I'll say that a lack of comments has never stopped me from
>> understanding how something works in LilyPond.
> 
> With all due respect, you don't even understand how the performance
> regression in your latest uncommented own submission comes about.
> 
> If you don't understand your own code right after writing it, how do you
> expect anybody else to understand it?
> 
> In an ideal world, people start writing the comments.  First about what
> they are doing, then it becomes more fine-grained, namely _how_ they are
> going to do it.  If this is done properly, the code itself can then be
> written by any code monkey.
> 
> And can subsequently be improved in passing by another code monkey who
> has a better idea how to cast the described problem into code.
> 
> With LilyPond, this does not work.  When looking at any code, you first
> need to analyze its _low-level_ operation.  Then you need to figure out
> all the steps it does and hope that they form a coherent picture
> _solving_ a problem.  Then you need to develop the problem description,
> and _then_ you can think about a reimplementation.
> 
> Take a look at <URL:http://codereview.appspot.com/6498070/>.  The code
> does not contain a _single_ comment.  You remove some things, juggle
> some things around.  _What_ is this supposed to do?  _Why_ is something
> happening that results in better output?  _What_ is happening?  Where is
> the explanation about why the code you removed could be removed?
> 
> Did you understand why the original code was in place?  Probably not,
> since the original code was not documented either.
> 
> The current LilyPond code base is a piece of unmaintainable crap.
> High-level crap, but crap nevertheless since it is, due to lack of
> comments, tied very hard to its original authors.
> 
> You are not responsible for the original heap of crap (which is rather
> high-quality because much of it is well-discussed two-person crap
> between two highly capable programmers who just did not bother writing
> down their discussions).
> 
> But by heaping new layers of undocumented undesigned unmaintainable crap
> on, LilyPond becomes manageable worse.  And more and more people think
> this is the way to do it.
> 
>> It is a single threaded program, so if you are not getting a result
>> you expect, it is easy with valgrind or gdb to find where things are
>> being set and why.
> 
> Very funny.  There is a simple rule: debugging code is twice as hard as
> writing it.  If you want to be able to debug your code, don't write at
> the limit of your capacities.  Coding is not composing.  The work is
> never finished, and other people need to be able to continue what you
> started.  And understanding foreign code is twice as hard as
> understanding your own.  If you are writing at the limit of your
> capacity, it will take somebody twice as smart as you are to work with
> your code.
> 
> Deal with it.  Good well-maintainable code is boring.  Its _design_ may
> be clever, and that design needs to be documented.
> 
>> I don't get what the underlying design is of which you speak - what of
>> it is undocumented?
> 
> You don't even write what your code is supposed to do.  _Nothing_ is
> documented.
> 
> Take <URL:http://codereview.appspot.com/6493073>.  In contrast to most
> of your code, there even is a comment.  Hooray.  Too bad it is
> completely unusable for figuring out the situation triggering the
> problem you try tackling.
> 
> How is _anybody_ supposed to maintain this kind of code when you are no
> longer around?  How are even _you_ supposed to maintain this kind of
> code in a year from now?  Or if you suffer a light stroke?
> 
> It is not like you are alone with that kind of attitude.  It is by far
> the most prevalent cause of projects going down along with their
> programmers.  Good code does not depend on the continued well-being of
> its programmer.
> 
> LilyPond is too large by now to sacrifice it to the egos of its current
> developers.  We can't afford that attitude.
> 
> We need to get into a state where LilyPond code, freshly submitted or
> historic, is reviewable in any meaning of the word that makes sense.
> 
> At the current point of time, we are on a fast road to
> unmaintainability, and we have gotten rather far already.  Graham's
> answer to that is to make it easier to get patches approved since rather
> few people have the confidence of attempting to work with the mess
> LilyPond is.
> 
> We can't go down that road forever.
> 
>> Rarely are tricky things like "set_property" used and when they are,
>> it is usually clear how and why (for example, the set_property calls
>> in Stem::set_stem_positions are used to set stem positions).
> 
> Sure.  But why do you set the positions?  What cases are you dealing
> with?  What changes affect the pure calculations, which ones the unpure
> ones?  What assumptions do you make that are provided by different code?
> All this is necessary to figure out how to make working changes.
> 
>> As I have relative little coding experience, I have no clue what is or
>> isn't good commenting practice.  I would be fine putting a comment
>> above Stem::calc_control_points saying "This function calculates the
>> control points for a stem" but I think all the information you need is
>> in the function name and in the function's logic.
> 
> Good comments don't repeat what the code already says.  They tell the
> story _behind_ the code.  _Why_ the code does something.  _How_ it fits
> together with other stuff.  _What_ the underlying problem is that the
> code tackles.  If you find somebody on your premises building a wall and
> ask him what he is supposed to be doing, you don't want him to explain
> how he mixes mortar, and how to put one brick on the next brick, and how
> to interleave bricks.
> 
>> What did take me time to learn is how everything fits together (what a
>> callback is, where things are triggered when, what happens before
>> what).  Some sorta automatically generated flow chart may help this -
>> I'm sure there are tools for that.
> 
> Get off it.  There are no tools for reflecting the high-level design of
> a program, unless that design is formalized and tools _particular_ to
> that design are written.  We do that for grob properties and interfaces
> and a few other things.  But the rest of the story is in comments, if
> anywhere.
> 
>> I've read through libraries that put paragraphs of comments above
>> single lines.  I spend so much time reading the prose that I can't
>> focus on the logic.  If variable names were called x0f00___EE7 in
>> LilyPond, I wouldn't be able to grock the logic, but I feel stuff is
>> pretty clear.
> 
> Yes, to the degree where you ridicule people telling you otherwise
> <URL:http://codereview.appspot.com/6498077/diff/21/lily/phrasing-slur-engraver.cc#newcode119>
> until proven wrong. <URL:http://codereview.appspot.com/6498077#msg7>.
> 
> And if you then reply with something like
> 
>   The time hike seems awfully expensive - there must be a way to
>   optimize it.  I'll post something that works when I have it and then
>   we can figure out how to optimize it.
> 
> it gives me the creeps.  You still think you are the only person in the
> world who should ever working in that area, so you still are not worried
> about actually documenting what you want to be doing.  Instead we'll get
> the next undocumented iteration which will at some point of time pass
> our review and submission processes because people just give up on
> trying to understand what you are doing.
> 
>>> It is also a timebomb for maintenance since changes might violate
>>> underlying assumptions.
>> 
>> The regtests catch this, no?
> 
> No.  Regtests are not designed for figuring out the weaknesses of your
> code.  Humans are, if you give them something to work with.
> 
> Our regtests are nice for pointing out huge blunders affecting unrelated
> areas.  But the _targeted_ regtests don't _construct_ corner cases like
> human reviewers can.  They test what the creator of them did think of at
> the time of creation.
> 
>>> And that is just talking about code that actually works for good
>>> reasons.  Quite a bit of code works since it has been prodded into
>>> not failing under those circumstances that tend to be tested.
>> 
>> I agree about the faulty code thing but not about the prodded thing -
>> I think once the right test case comes up one can almost always find
>> what is failing and why.
> 
> Reality check: take a look at our open issues.
> 
>>> It is easy to make it easier to meddle with LilyPond code.  The low
>>> number of contributors is not due to our toolchains.  It is because
>>> few people are comfortable poking around in the dark.  And for good
>>> reason.
>> 
>> I agree that LilyPond is gargantuan, but I think the main impediment
>> to understanding how things work is a global lack of documentation
>> about how things fit together.
> 
> And you are improving this just how with your "not even the commit
> message tells what I am doing here, let alone any comment" kind of
> patches?
> 
>> This could be put in the CG.  If we can decide that certain basic
>> elements of LilyPond's design will be frozen, this is worth doing.
> 
> Horse radish.  What is worth implementing, is worth documenting.
> 
>> I am completely on the same page with you about wanting to make
>> LilyPond easier to contribute to.  Statements like "There is a lot of
>> code that X" or "Quite a bit of code Y" may be true, but it is very
>> tough to tackle the problem w/o concrete examples.
> 
> What's wrong with pointing out the complete absence of any commented
> rationale in your submissions?
> 
> When I point out where particular code passages of yours are missing a
> rationale, how about putting it _in_ the next iteration instead of doing
> something else and giving me the "silly old fool" treatment?
> 
> Yes, I _am_ a silly old fool, but I have written a lot of code in my
> lifetime.  Yes, this includes complex stuff written in assembly language
> where the only comments were accumulated cycle counts (in order to keep
> game timing, partly even including sound frequencies, accurate and
> deterministic without relying on timer circuits).  And some of this
> stuff remained astonishingly clear and well-designed when looking at it
> decades after writing it.  But nobody except myself is going to maintain
> it, and what you don't see are wastepaper baskets full of drafts and
> diagrams.  They are inherent in the code, and that _does_ help in
> understanding the code because it is _not_ just written from scratch,
> but an actual implementation of a long-planned coherent design.
> 
> And the source code had to be stored on magnetic tape at 2400 bps, and
> it had to fit in something like 50kB at the most.
> 
> We don't have those kind of restrictions any more.  The design, or at
> least its constraints and how and why the current code meets them,
> belong in the source code.  Not implicitly, but explicitly.  Code is
> written once, and in any healthy project, read multiple times.  So it is
> the task of the writer to make efficient use of the readers' resources.
> When reading is faster than figuring things out, and this is pretty much
> _always_ the case where the relations with other code pieces and the
> choice of particular non-trivial data structures are concerned, a
> comment saves even a single reader more time than it costs the writer.
> 
> You probably know the joke about the experimental physicist approaching
> the university dean for more money.  And the dean says something like
> "Money, money, money.  Why can't you take a leaf from the
> mathematicians?  We just provide them with pencil, paper, and a
> wastebasket, and that's all they need.  And the philosophers don't even
> need a wastebasket..."
> 
> Regarding the expensive storage area for comments, some of the old code
> might have been written by mathematicians.  Which is bad.  What is worse
> if you figure "oh, this makes all sense, so I'll write without comments
> as well and make equal sense" because you never got to see the
> wastebasket.  Han-Wen and Jan explained the code to each other until it
> made sense.  Too bad they did not write the explanations down.
> 
> By writing comments, you can at least explain the code to yourself.  If
> you find that hampers your flow significantly, it is likely that you are
> not writing code easily explained to anybody.  Including the computer.
> 
> I may be a silly old fool, but I've grown old enough to gain the
> arrogance to consider code for which I don't understand its purpose
> after reasonable consideration is bad, because I am far enough to the
> right of the bell curve on understanding computers and programs that
> code that is beyond me is for all reasonable purposes beyond meaningful
> maintenance, possibly even by its own author but most certainly by an
> average programmer.
> 
> -- 
> David Kastrup

David,

I appreciate your comments regarding comments and other things.

My code tends to be rather sparsely commented (if at all) because, as you state 
above, there is an internal logic to LilyPond created by the original authors 
that is uncommented but consistent, and once one understands that, one's work 
fits into that logic.

As for the valgrinding, a lot of my work in LilyPond is knowing what I want to 
do on thing X but not knowing how thing X is linked to other things in the code 
base.  For example, I'll know how grob X uses grobs to find its extents, but 
the reverse is much more difficult - knowing what grobs use grob X to find 
their extents.  So it's true that when I start solving a problem it is often 
well beyond my understanding of how a single grob is interconnected to all 
other parts of LilyPond.  If I let this stop me, I wouldn't have done any work 
on LilyPond.

I don't mind going through the code, file by file, and writing comments 
everywhere.  I understand most of it and if you think that'd lead to better 
maintainability then it is worth it.  I'll try to do 3-4 files a week.

And I definitely didn't mean to not respond to one of your suggestions - I 
forgot to get around to writing that comment you asked for as I was distracted 
by other things in the patch.  Nor do I mean to ridicule you - the Strong Bad 
Tandy 400 comment was because I don't think there's a real efficiency tradeoff 
(I did some tests and in fact there isn't).  I do take your suggestions very 
much to heart, however, and am experimenting w/ a new way of going about it 
that uses only vectors and is much clearer.

Cheers,
MS