Re: [boost] Coding guidelines - comments

From: David Abrahams (david.abrahams_at_[hidden])
Date: 2001-07-01 08:55:49

----- Original Message -----
From: "Paul Baxter" <paul_baxter_at_[hidden]>
To: "Boost List" <boost_at_[hidden]>
Sent: Saturday, June 30, 2001 10:36 AM
Subject: [boost] Coding guidelines - comments

> A few minor comments on the coding guidelines.
>
> Firstly, thanks Dave and Nathan for making this available.
>
> a) Section 1 - Perhaps add guideline about when to use #include
> "myheader.hpp" instead of <myheader.hpp> Just to clarify system/library
> headers vs user's headers.

Actually I don't know what the rule should be. After working on the build
system, I find that compilers have inconsistent (often configurable) rules
about what they'll do for #include "..." vs #include <...>. The only thing
that seems consistent is that #include "..." may search more places than
#include <...> (e.g. the directory of the #including file).

> b) Example after 1.8
>
> Confusing use of Rectangle and rectangle

Yep, I missed that one during a global naming convention conversion, thanks.

> #endif // should end with HPP_ not H_
>
> c) 2.1/2.2 Confusing to use vhdl_ams_parser as both examples
>
> Maybe use HdlParser in the second?

How about XmlObject?

> d) Consider having a general section for layout before specifics of
> definitions/blocks etc.
>
> e.g for no tab rule
>
> Ident in 2/4 space blocks (except 'public:', 'private:' I guess)

Your suggestion is unclear to me. Would you care to submit a proposed patch?

> e) 4. Spacing of definitions and declarations
>
> Resolve: In various places you use 1 space, two spaces, three spaces or
> four. I find this confusing.

I agree. Unfortunately, you don't get the same indentation help when editing
html as when editing C++ ;-(.
I cleaned up a few spots, but a patch from you with corrections would be
very helpful.

> e.g. hpp/cpp files at end of section 4
>
> Perhaps the spacing lost something in translation to HTML?
>
> Whatever the result I'd like a consistent policy between the guidelines
and
> the examples.

You are right.

> f) Why
> namespace altra {
>
> instead of
> namespace altra
> {
>
> when you use
> class catalog
> {
>

Because it's not uncommon to have to open or close several levels of
namespace nesting at once, and separating them vertically obscures the final
namespace we're trying to open.

namespace boost
{
namespace python
{

class tuple
{
 public:
    tuple();
    ...
};

} // namespace boost::python
} // namespace boost

Anyway, that's the justification, but I'm not wedded to it.

> g) 6.5 Perhaps qualify this by acceptable reuse of loop variables and
> possibly pointers/iterators i.j.k, p and q  especially since they often go
> out of scope after the end of say the for loop (??)

Maybe it needs to be clarified, but I meant that the /variables/ shouldn't
be reused. I have no problem with reusing variable /names/.

> h)
> >
> 6.6. <part snipped>
> Declaring a value-parameter const has no effect in a function declaration
> and is meaningless to the caller, so leave const off here - use it at the
> point of function definition instead.
> double f(double x, double y);   // declaration
>    ...
> double f(double const x, double const y)   // definition
> {
>     // x and y don't change in the function body, so
>     // we know the original values are always accessible
> }
> <
> Not sure I like this. Prefer to see const in the declaration as well.
(Must
> admit I didn't even realise this was permissable)

The const doesn't tell the reader of the declaration anything useful at all.
(s)he already knows the actual argument won't be modified if it is
pass-by-value. To avoid adding meaningless tokens, const for value
parameters is banned from interfaces.

> i)
> > 7.1 Use ``//'' to delimit comments in lieu of ``/*.*/''  <
> Rationale for this please.
> I guess the issue for me is whether the source/header should sometimes be
> used to document a design as well as clarify details of its coding
> implementation.
> If it is sometimes permissable to provide long comments discussing the
> design, the use of multi-line comments is convenient and does not obscure
> the text as much as // every line.
> (Dare I say that most people now use syntax highlighting to avoid some of
> the problems with missing the closing */)
>
> In particular the use of class documentation tools like doxygen although
> possible with '//' is far more convenient with the /* */ form.
> I'm not going to get too upset if you disagree, but how can '//' at the
> start of each line help when reading a multiline comment.

I don't feel strongly about this, but I also have no trouble with seeing
'//' everywhere. The problem with /*...*/ is that people sometimes try to
comment out a long section by enclosing it in /*...*/. If it already
contains /*...*/, then boom (especially if it was long enough that you can't
see the inner /*...*/) Some people have suggested allowing /*...*/ for TEXT
comments, but you can embed code examples in text. Now the nesting problem
is back.

> j) 12.2 and 12.4 seem to conflict.
>
> General comments
>
> Include a version or issue date for the guidelines document to help keep
> track of any revisions.

OK, I guess I'll check this into CVS

> The document covers a lot of ground and I am concerned that a few parts of
> later sections require considerable in-depth knowledge of c++ to be of
use.
> e.g. Discussions of basic and strong exception guarantees are quite
advanced
> and may be better served by references to other works.

I feel they're important to /anyone/ writing C++, especially at the boost
level. It may be worth adding additional references. Suggestions?

I just noticed that I have the strong guarantee as the default. I think it
should be changed so that the basic guarantee is the default, and the
strong/nothrow guarantees are available where explicitly documented.

> Whilst I understand it is not practical to go into the background and
detail
> of some of these statements, it may be necessary for programmers who are
not
> as familiar with recent C++ facilities to dig a little deeper to
understand
> things. Adding a short bibliography might be enough?

That might help.

> I feel the basis of a very useful book here :)

Thanks. Nathan and I are discussing the appropriate license.