Re: [boost] Re: Suggested coding guidelines

From: David Abrahams (david.abrahams_at_[hidden])
Date: 2001-07-06 16:50:35

[Nathan, one of these could use some interpretation from you]
----- Original Message -----
From: <williamkempf_at_[hidden]>
To: <boost_at_[hidden]>
Sent: Friday, July 06, 2001 1:51 PM
Subject: [boost] Re: Suggested coding guidelines

> I'm going to throw my comments in on this document.  First, let me
> say that most of the guidelines are the very same ones that I use for
> most of my coding (when other guidelines don't dictate otherwise).
> So I very much like what's here.
>
> Now for specific comments:
>
> 1.6  Does this bar comments from being placed above the #includes and
> #include guards?

I guess so, though I think its unintended. If you'd like to supply new
wording allowing initial comments I'm not opposed.

> 4.1  Should the guidelines specify the number of spaces used for an
> indentation level?  If so, should it be consistent for all
> indentations?

indentation to indicate namespace scoping can use up a lot of real-estate.
Since there's almost no information at the level of indentation where the
namespace is declared, it isn't worth wasting space on. Also, indentation is
only useful if you stand a reasonable chance of seeing the indented and
unindented stuff at the same time. Namespaces tend not to have that
property. That's why it's 2 for a namespace and 4 everywhere else.

> 4.2  Should the example (code that reads "template <class T> void f
> (T);") follow this rule?

Yes, thanks.

> 4.4  Why?  As long as all lines but the first in a multi-line
> definition is indented another level the blank lines add little
> benefit, and in fact can detract from the rest of the code.

IMO vertical density makes reading code difficult. If you stick several
multi-line definitions next to one another, things tend to get too dense.

> 4.5  See 4.1.

Consistency of indentation of continuation lines is less important than the
other one. I didn't think it was worth mandating.

> 4.6  Wording should indicate this applies to multi-line definitions.
> As written it means that ALL function definitions should be multi-
> line.  Also, see 4.1.

I got this guideline from Nathan. I always read it to mean that all function
definitions with arguments should be multi-line. Of course I don't follow
that practice myself. I like your reading better. Nathan, what do you have
to say about this?

> 4.7  Again, why?

Same reasons.

> 4.8  Why?  Also, see 4.1.

It looks nice ;-)
I used to indent them zero spaces, but worked somewhere that they were
always indented 1 space. I grew to like it. It shows that the class body
isn't over yet. I'm not wedded to this one.

> 4.9  Not sure I like the inconsistencies.  Also, the example uses
> both template<> and template <>. Should there be a rule for this?
> Even if not, the examples should be consistent.

It should be template<>; my mistake.

> 5.3  See 4.1.
>
> 5.7  I find this a little restrictive with the reasoning being
> questionable.  That said, I can live with it.

Here's an example of the alternative:

    if (x < 0)
        x = 0;
    else
    {
        for (y = 0; y < x; ++y)
            do_something();

       do_some_other_thing();
    }

My eye says that the x=0 goes with the 'if' clause and the rest is a
separate entity like a struct declaration or a while loop. This is an
aesthetic consideration; others' taste may not coincide here.

> 5.8  See 4.1.
>
> 6.7  This makes the code read less like english.

Depends whether you think const is an adjective or a noun. I think it's a
noun ;-)

> Also, the comment
> about being consistent with compiler error messages is, frankly,
> misguided.  We can't predict how compilers will display this sort of
> message.

Except that compilers have to go out-of-their way to generate "const char"
instead of "char const" when they also have to generate "char* const". If
you pick a simple algorithm to translate an internal type representation
into valid C++, it will always put the const afterwards.

That said, I still have a hard time adjusting to writing "char const". OTOH,
if you have to pick a rule, it should be a consistent one. The alternative
is no rule: some people write "char const" and others write "const char". Do
we care?

> 7.2  Should read "comments refer to code to the left on the same line
> or below them."

Changed to "Long comments begin on a separate line from active code, and
refer to the code below them. Comments of more than one line are separated
from other code by a blank line above. "

> 7.3 & 7.5  Don't quite work well together.  Combining them will
> better explain that public and protected interfaces should be fully
> commented at the point of declaration.

Please suggest new wording.

> 7.3 & 7.9  Though they technically don't conflict, they lead to
> confusion because of their differing emphasis on accuracy or brevity.

Yes, they are explicitly recognized as conflicting goals. Do you have a
suggestion?

> 7.12  It would be helpful to have a document listing the proper C++
> terminology and improper variants.

Would you like to write one?

> 7.15  Commenting out code should be avoided.  Yes, there are times
> when this rule can be broken, but generally it's better to rely on
> the configuration management tool (CVS for us) for recalling code
> that's been removed.  Commented out code simply makes reading more
> difficult.

Changed to:

Comment out code using `` #if 0...#endif'', or ``//'', not ``/*...*/''
comment notation. Avoid checking in code that's been commented out. Since it
doesn't get tested, it will likely not make any sense tomorrow even if it
makes some sense today, .

> 8.1 & 8.2  Though use of public and protected data members may be
> questionable from a design stand point, this is a coding guidelines
> document not a design guidelines document. (Boost should have both,
> but the coding guidelines should be careful to not discuss any design
> guidelines.)

Even if I agreed that it would be a good idea to separate the documents (I'm
not sure I do), I am definitely unwilling to invest the effort required. If
you would like to generate a consensus for two documents and do the editing,
though...

> 8.3  Also a design guideline, though the rule about friend being
> close to code meant to be exposed is worth retaining.
>
> 8.4  Why make an exception for operator()?

It's not an exception for operator(), it's for simple function objects.
These are just simple functions in disguise; separating interface from
implementation is more trouble than it's worth in my experience.

> The reasoning given just
> begs the question since the word "often" is used.

Where is the word "often" used???

> What about the other cases?

I think the wording is clear: there are no "other cases".

> What about in template classes?

"Function definitions in the class body are forbidden"

> 8.7  Why?  The grouping and commenting I can agree with, but leaving
> off the virtual can lead to confusion for readers not fully versed in
> C++.

The rationale is in the discussion section below:

"Writing `virtual' on the declaration of a virtual function override is
neither neccessary nor sufficient to document where it comes from and which
code it interacts with. Leaving the keyword off in derived classes reduces
clutter and thwarts the temptation to treat it as sufficient documentation"

> All of 9 & most of 11, 13, 14 and 15 should be in a design guideline
> instead.

Are you volunteering?

> 12.2  What does this mean for Boost where the convention is to put
> all code in the boost namespace?

I think the emergent rule for boost is: separate domains go in their own
sub-namespaces. General-purpose code goes in boost. Probably 12.2 should be
changed to reflect that fact.

-Dave