Re: Coding practices

[address@hidden]

On 15 févr. 2013, at 23:44, David Kastrup <address@hidden> wrote:

> "address@hidden" <address@hidden> writes:
> 
>> Hey all,
>> 
>> After reading over David's most recent e-mails to the list, it is
>> clear to me that there is a coding-practice debt in LilyPond.
>> Standards of how and when to do certain things are not shared across
>> all programmers, and things that seem commonsensical for some do not
>> seem so for others.
>> 
>> Rather than letting this continue to be problematic, I'd like to
>> establish a sort of GLISS for LilyPond coding style.  We would discuss
>> certain conventions in the codebase to see where commonalities lie in
>> the codebase and where things could be made to look more similar.
> 
> That's the wrong approach for coding guidelines since it assumes that
> "commonalities in the code base" suggest a reasonable coding practice.

I don't understand what you mean here.  There are currently many instances of:

Grob *me = unsmob_grob (smob);

Surely you are not suggesting that it is reasonable to do:

Grob *susan = unsmob_grob (smob);
Grob *barf = unsmob_grob (smob);

depending on the proclivities of the programmer.  I think that commonalities in 
the code base are essential - it is the lack of these that causes the confusion 
in variable names that we're talking about.  There have been several reviews 
where you get ornery about a variable or function name that I use that is in 
fact the copy of another variable name used in the same way at another point in 
the code base.  I realize that the message you're driving home is "just because 
it is done poorly in one place doesn't mean you should keep doing that", so 
what I'm saying is let's have it done well and similar everywhere.  Grob *me is 
the most ubiquitous naming convention in the code base (and even that is not 
followed 100%).  One thing I would like to see, for example, is :

Real me_padding = robust_scm2double (me->get_property ("padding"), 0.0); // 
variable names for extracted properties are always attached to the grobs they 
come from
Real me_padding_scm = me->get_property ("padding"); // for SCM variables, _scm 
should always trail
extract_grob_set (me, "elements", me_elements); // same applies to 
extract_grob_set

SCM me_vertical_skylines_scm = me->get_property ("vertical-skylines");
Skyline_pair *me_vertical_skylines = Skyline_pair::unsmob (me);
Skyline_pair me_vertical_skylines_copy (*me_vertical_skylines); // the name 
copy in a variable indicates that things are copies

I know that this does not explain the "why" of the code in the way you are 
suggesting via comments, but in terms of the "what", it makes it a lot more 
clearer which invariably helps understand the "why".

Ditto for certain common actions.  We shift and raise skylines often by x and y 
offsets using a common grob.  Why not have a common practice, all over the 
code-base, using:

shift_and_raise_skyline (Skyline &skyline, Axis axis, Grob *me, Grob *common_x, 
Grob *common_y)

Commonalities in the code base are, in my opinion, a necessary but not 
sufficient thing to have good, readable code in LilyPond.

> 
> The vast majority of the LilyPond code base is not coded with
> maintainability or transparency to third-party programmers in mind.

Yes.

> 
> If you want an example of reasonable documenting and coding practice,
> take tex.web (it is public domain), run it through weave and pdftex and
> peruse significant extracts of the resulting PDF.
> 
> That's reasonable coding and documentation practice.  The design is
> post-sixties, Pascal (of that day) is not a modular language, and there
> are too many shortcuts and "smart" programming practices for having a
> well-extensible code base.  But the documentation level and style is
> pretty much on spot.
> 

Ok.

Cheers,
MS