Re: [lmi] [lmi-commits] master 783f4dd 5/9: Consolidate and revise documentation

[Greg Chicares]

On 2018-04-05 13:42, Vadim Zeitlin wrote:
> On Wed,  4 Apr 2018 21:27:37 -0400 (EDT) Greg Chicares <address@hidden> wrote:
> 
> GC>     It seems generally best to comment a member function in one place 
> only:
> GC>     at its point of definition.
> 
>  Would it be possible to discuss the rationale for this rule? Personally, I
> find it very inconvenient to have comments explaining what the function
> does (as opposed to how it does it) near its definition, as opposed to its
> declaration, which is where I'm looking if I have any questions about the
> function (e.g. by jumping to a tag in Vim). Of course, the fact that it's
> very unusual doesn't mean that it's necessary wrong, but it's still worth
> mentioning that lmi is the only project I know of which doesn't put
> comments near the function declaration and so I'm quite sure that I'm not
> the only one who is confused by not finding them in the header.

I've heard others recommend describing
 - _what_ a function does, near its declaration; and
 - _how_ it does that, near its definition.
I don't like that guideline because:

 - I'm not convinced that _what_ vs. _how_ is a generally useful distinction.
Over time, an initial _what_ vs. _how_ partition is likely to degrade anyway.
In practice, the header easily ends up including just the first line of the
implementation's block comment.

 - Every function would be documented twice, with different parts in different
files. Flipping back and forth between two files is harder than focusing on one.

 - I alter implementation more often than interface, and most often I change
the implementation of a single function. The most convenient place to look
for the documentation is next to the function I'm modifying and nowhere else.

 - Comments in one file become outdated, and no longer reflect what's in the
other file--or even grow to contradict it.

 - If code and commentary are both revised together in the implementation,
then only the implementation need be recompiled. Header changes require more
recompilation.

>  Finally, there is a question of consistency: documentation comment of a
> class is, necessarily, located before its declaration, as there is no
> definition for it.

I could of course say that "class C {...};" is a definition (as well as a
declaration), so "comment the point of definition" is perfectly consistent.
But let me also try to address the substance of what you say...

> Why should it be different for the functions?

Function comments often want to be long. Headers want to be terse, so that
the whole thing can be gotten at a glance. Take 'input_sequence.?pp' for
example. The class definition (declaration) is lines 189-233 in the header:
45 lines in total. Suppose I move the block comments from the implementation
into the header (here I'm not counting the blank lines we'd surely add):
  7 comment lines: ctor(vector<string>)
 14 comment lines: ctor(vector<double>)
 19    "      "  : initialize_from_vector()
 43    "      "  : canonical_form()
Then the header would be several times as long. No longer could I see all
the public members on one single 26-line screen (given my myopia, that's all
that fits when vim is maximized), and all the private members on another.
Comparing all the declarations in either set would require moving my fingers
rather than just my eyes. So that idea doesn't work.

That's why, as I remarked above, requiring a comment on every function's
declaration so readily turns into a rule like "copy the implementation's
first comment line into the header". Thus:

    /// Construct from vector: e.g, a a a b b --> a[0,3); b[3,4).
    explicit InputSequence(std::vector<std::string> const&);

    /// Construct from vector: e.g, 1 1 1 2 2 --> 1[0,3); 2[3,4).
    explicit InputSequence(std::vector<double> const&);

Perhaps you'd find that helpful; its value is greater than nothing. It's
better than copying in whole comment blocks: it only makes the header
twice as long. Is that a good compromise, then? No. If I change the way
one of those functions works, I'm probably going to change the header
only once, and then change and test the implementation several times.
It's takes effort and discipline to keep the comments synchronized, and
that's not what I'm focused on while trying to change the implementation.
And, as observed above, it makes the header longer and balkier.

Let me also use that "1 1 1 2 2 --> 1[0,3); 2[3,4)" ctor to illustrate
a point made above--its block comment contains:

/// This is used, e.g., when interest rates obtained from an external
/// source vary from one year to the next, and it is desired to use
/// them as lmi input. It might seem that...

Which question does that answer: _what_, or _how_? Neither. It answers
_why_. So where does it belong: header, or implementation?

>  I really wish this rule could be changed and the documentation comments
> could appear at the point of function declaration instead. It would
> definitely make reading them much more convenient for me personally and I
> just don't see any drawback of keeping them there.

I hope I've explained the drawbacks, which I regard as fatal.

>  What am I missing? Why does lmi do it differently from all the other C++
> (and even C) headers in existence?
Surely that's a bit hyperbolic. Look at page 539 of the C++17 standard
(N4659.pdf), which is [pairs.pair]. This is header perfection. All three
"template<class U1, class U2>" ctors appear on three successive lines.
If I had an instrument measuring my eyeball movements, it would have
recorded them moving dozens of times over the arguments alone, in an
interval of just a few seconds. I can get it all at a glance: I can see
the complete scope and limits immediately.

Or look at class crc_basic here:
  https://www.boost.org/doc/libs/1_66_0/boost/crc.hpp
of which this section is typical:

    // Internal Operations
    value_type  get_truncated_polynominal() const;
    value_type  get_initial_remainder() const;
    value_type  get_final_xor_value() const;
    bool        get_reflect_input() const;
    bool        get_reflect_remainder() const;

Just a two-word comment describing a group of member functions, along
with the declaration of each. _What_ does each do, and _how_ does it
do it? For that, you look elsewhere. The header is a synopsis only.