Re: [boost] Suggested coding guidelines

From: Greg Chicares (chicares_at_[hidden])
Date: 2001-06-30 20:34:17

David Abrahams wrote:
> 
> Greg Chicares wrote:
> > | Table of Contents
> > |   6.Declarations and initialization
> > |   7.Comments and Documentation
> >
> > Suggest capitalizing only the first letter in items 7, 8, 10, 14.
> 
> Since these are section titles, shouldn't I capitalize all (important)
> words, thus changing item 6?

I think any consistent style is fine.
If 6 changes, then maybe 1, 3, 4, 5, 9, and 11 should too.

> > | 1.2 #ifndef RECTANGLE_DWA050499_HPP_
> >
> > Could I persuade you to use ISO8601 dates instead? I have to think
> > about whether this means
> >   RECTANGLE_DWA19990504_HPP_
> > or
> >   RECTANGLE_DWA19990405_HPP_
> 
> I'd be happy to go with ISO861, but the date is just for uniqueness; it is
> not supposed to be read except by the preprocessor.

A feature isn't truly cool until someone uses it for some purpose the
inventor didn't anticipate. This meets that criterion in a way that
you might not intend. As long as a date is there, people will
  read it,
  grep/sed/awk for it,
  believe/trust/depend on it,
and, since this is a coding guideline, ask questions like "do you
update it when you change the file?"

Have you ever run into a problem using just something like
  RECTANGLE_HPP
anyway?

In place of
| Incorporate your initials and the date as shown in this example. 
I'd suggest something like
  Form the include guard name from the filename, replacing periods
  with underscores. For additional safety, you might wish to add
  your initials and the date as shown in the example.

> > | 1.5 Of course, this technique's effectiveness is mitigated by the
> > extent to which (generic) code appears in header files. [context:
> > include class header first]
> >
> > I'm not sure I follow this. Does 'generic' mean 'templates', or just
> > something like 'reusable'?
> 
> "templates", but it's parenthesized. Inline functions also mitigate it.

OK, implementation in header <--> less chance of unnoticed dependencies.

> When there's no corresponding .cpp file, what do you do?

Doesn't matter. Given a header, this rule doesn't require the existence
of a .cpp file that must include that header first.

Take lexical_cast.hpp as an example. There's no corresponding .cpp file
for the rule to apply to. This header never gets included first
(except in lexical_cast_test.cpp, which is a special use). It's the
limiting case: complete mitigation of the technique's effectiveness,
by removing the problem the technique would solve--a good thing.

> > | 3.6 return is not a function; do not put parentheses around the value
> > being returned. This also goes for throw.
> >
> > Would you say the same for sizeof(unary-expression)?
> 
> No. You usually need parens for sizeof (e.g. sizeof(char*))

Only for sizeof(type-id), not for sizeof(unary-expression).
But forbidding unnecessary parentheses in the latter case
goes beyond ordinary pedantry, so the guideline's probably
better left as is.

> > | 7.15 Comment out code using `` #if 0&hellip;#endif'', or ``//'', not
> > ``/*&hellip;*/''comment notation.
> >
> > Under these guidelines, are '/*...*/' comments ever OK? If so, when?
> 
> I guess not.

The guideline's OK, because it doesn't literally forbid

  /*
    block comments like this
    which may be many line long
    and contain characters like ' (apostrophe)
  */

although I'd rather it said something like
  'Use /*...*/ only for multiline text comments.'
gcc is strict about the undefined behavior of

  #ifdef 0
    won't work with gcc
    what preprocessor token would the lone apostrophe belong to?
  #endif

and recommends /*...*/ instead for *text* comments; I just
don't want to see that usage deprecated.

> I wrote this standard for people who didn't have a 
> lot of C++ experience.

Depends on your definition of "a lot"! I'd say these guidelines assume
considerable knowledge. That's a strength--it keeps them from being
verbose, and encourages the less knowledgeable to figure out what they
don't know yet. I plan to use this as a learning and training tool.

> > | 11.4. Use assert() liberally
> >
> > Is 'assert()' here shorthand for 'some assertion macro or template'
> > including BOOST_STATIC_ASSERT or Assert in C++PL3 section 24.3.7.2?
> > Or does this imply that the macro in <cassert> is to be preferred?
> 
> Right now, it means the one in <cassert>. I agree that BOOST_STATIC_ASSERT
> should be recommended for compile-time assertions.

Yet see the comment on polymorphic_downcast in boost/cast.hpp
 // WARNING: Because this cast uses assert(), it violates the One Definition
 // Rule if NDEBUG is inconsistently defined across translation units.
An assertion facility that doesn't depend on NDEBUG shouldn't
be a violation.

> > | 14.6 `C'-style casts are safer than reinterpret_cast<>. [...]
> > Does this guideline mean that reinterpret_cast should never be used?
> 
> Almost.
> 
> > It has the advantage of looking ugly and being easy to grep for.
> 
> I've never needed it. Have you?

Pass a string via windows messaging, which wants an integer type
  ::SendMessage(*fw, DestID, 0, reinterpret_cast<long>(s.c_str()));
Catch the long int elsewhere and treat it as a char*
  SomeControl->SetText(reinterpret_cast<char*>(LParam));

I just grepped all the code I've written here over the past
several years, and aside from windows stuff, I find only two
uses of reinterpret_cast. Oops...neither of them looks right.
The point is I could find them easily. It almost screams
"nonportable, possible error" and is safer in the sense that
it attracts scrutiny and is likely to be removed if bogus.
If my two uses (windows aside) were in current production
code, I'd reevaluate them right now.

So I'd agree that reinterpret_cast should almost never be used,
yet add that C-style casts should be used even more rarely.