Review: Boost Coding Guidelines

From: Ronald Garcia (garcia_at_[hidden])
Date: 2001-07-18 09:24:12

My Review of the BOOST Coding Guidelines

I think that the Boost Coding Guidelines should be accepted into
boost.  I think that as it stands, it's headed in the right direction,
and I commend Dave for all the work he's put into it. I am curious
though about applying the same review process for libraries to
documents. It's easier to accept a library before it is in its
finished form than a document.

First I would like to note that I haven't followed all of the previous 
conversation about this document, but I did review the current
iteration in its entirety.  If some of my comments relate to decisions 
previously iterated upon, perhaps further explanation of rationale
should be added to the document. 

Specific Comments Relating Specifically to the Guideline:

In the introduction, perhaps the statement "for those cases where
authors have no preference or are willing to change." should be made
boldface. That might further please those who are afraid of coding
standards. In the second paragraph, perhaps further emphasis could be
placed on this as well.  The second sentence could be something like
"All the same, the authors of these guidelines intend that they be followed 
completely, if at all."  

As far as broad principles, perhaps "compatibility with the
current standard library style" should be mentioned.

1.2 - I'd suggest noting "For example,
you may use a GUID, or you can incorporate your initials..."  The
method of making unique header guards specifically is not important so 
long as it doesn't interact with other methods in a way that affects
that uniqueness.

2.3 - I think that Concept names in documentation should have spaces
between words.  The SGI STL web pages use this notation, and the
standard does the same as well, though they differ on
capitalization.  The STL docs capitalize concepts 
(eg. "An Associative Container is a variable sized Container that..."),
whereas the standard does not ("If an algorithm's template parameter is
RandomAccessIterator, RandomAccessIterator1, or RandomAccessIterator2, 
the actual template argument shall satisfy the requirements of a
random-access iterator (24.1.5).")

2.5 - what do you mean by compound words?

2.14 - Perhaps prepend the bold part with Do not use ("Do not use names
containing double underscores.").  Seems to fit in better with the
other clauses starting "avoid..."

4.3 - I would argue that return type should go on a separate line from 
the function name, but at the same indentation level. This supports
descriptive type names, not to mention member functions whose return
type is generative (these can get long).  With the function open brace 
on its own line, this style is very visible to the eye.

4.6 - I would add more to this clause.  As far as rationale, it would
be to encourage the use of detailed type and variable names, but NOT
to comment each parameter.  Using two-to four spaces ensures
consistency when class scoping brings parameters out near the right
column.

4.8 - why?  I ask because my access specifiers are at the same scope
as the opening brace of the class itself.  I only do this because my
editor of choice does this.  Is there a rationale?

5.1 - I would like to see more rationale added to this one. Mine?
Clear separation of blocks. Perhaps a mention of vertical screen real
estate on modern development systems as well (then, "if you find that
your functions still take up too ...")

5.3 - Why four versus two?  I could argue that this can interact with
long variable/type names, suggesting that two might be better than
four, Is something currently indented two spaces that would be easier
to see if blocks are indented four?  I know this is nitpicky, but I
would really like to see more rationale where there can be.

5.6 - Doesn't dictate whether single-line clauses should NOT be
blocks.  Is there any stance on this, or is it a case-by-case,
developer-by-developer decision?

6.2 - Mention that 6.1 keeps you from shooting yourself in the foot on 
this one.  Perhaps also mention c++'s focus on types versus c's focus
on values (ie. in C, "int *p" says "*p is an int" versus C++'s "int*
p" saying "p is a pointer to int").

6.5 - typo "variables from being used, and is enables..." to "and
enables"

Section 7 -I think more discussion of commenting versus documenting is
in order. If a public interface is documented separately from the
code, I don't think it should necessarily be documented in the code as
well.  This seems to be an issue throughout this section.

Section 7 Discussion - IMHO, "comments" are meant to be comments, not a
mechanism for blocking out code.  That is more of a side effect.  But
perhaps this is more of a religious issue.  I think that #if 0/#endif
expresses intent better

8.3 - I would consider making an exception to this rule.  A template
that is a friend of itself shouldn't have to access members through
member functions (ie, implementing const_iterator vs iterator by
templatizing the pointer type).

8.4 - I think a mention of class templates is probably in order here
since some compilers puke on this stuff.  Perhaps an addendum
covering these kinds of things might be a better place for 
it though.

11.16 - Others mentioned this as well. I think this should go.
Especially in the face of templates, the meaning of this statment
becomes pretty unclear, and doesn't necessarily fit in with coding
guidelines anyhoo.

Section 14 comments

Mention that C casts can be hard to find compared to c++ casts.

ron