Re: [Fresco-devel] Coding guidelines again

Nathaniel Smith wrote:

>  - Return type and function name on separate lines
>      void
>      foo(int bar)
>     rather than       
>      void foo(int bar)
> 
>    Justification: Return types in C++ can be pretty long; this reduces
>      wrapping and is easier to read to boot.

can't we make this optional ? 'void foo(int bar)' should really be
a one-liner. I agree that wrapping can help if the whole gets too long
so we have to wrap either the argument list or the return value or both.
But if it fits into a single line, I don't think we should mandate wrapping.


>     If you have an initializer list, put the ':' after the closing ')'
>     of the parameter section and start a new line.  If the
>     initializers all fit on a single line, put them on a single line;
>     otherwise put each on a separate line.  [[FIXME: rewrite this
>     rule, and break it into multiple sub-rules]]
>     So:
>      class Foo {

uhm, shouldn't that read

class Foo
{

according to the proposed style ? At least that's what I'm using and it 
is consistent with the bracing styles mentioned elsewhere...



>          // Correct:
>          Foo() :
>              my_bar(0), my_baz(0)
>              {};

no semicolon please !

>          Foo(int super_special_long_named_variable) :
>              my_bar(0),
>              my_baz(new baz_type(super_special_long_named_variable))
>              {};
what is wrong with

            Foo() : my_bar(0), my_baz(0) {}
            Foo(int super_special_ling_named_variable)
              : my_bar(0),
                my_baz(new baz_type....)
            {}

note the braces being on the same level as 'Foo', while the initializer
list is indented.

>          int
>          bar() { return my_bar; };
>          void
>          baz_incr()
>              { if (my_baz) my_baz->references++; };
>          void
>          quux() {};
>          inline void
>          blargle();
>          // Incorrect:
>          void
>          baz_incr2()
>          {
>              if (my_baz) my_baz->references += (::should_do_it ? 1 : 0);
>          }

I find this significantly harder to read than one function declaration 
per line (i.e. no wrapping of return types).


>          ...
>      };
>     Anything longer should go after the class declaration:
>      inline void
>      Foo::blargle() {
>          ...
>      }

again, what about:

inline void Foo::blargle()
{
...
}

i.e. (aside the return type not being wrapped) braces consistently
wrapped with other blocks.


> 
>    Justification: Don't clutter up declarations with code, unless the
>     functions are so trivial that it doesn't matter.

right

> 
>  - Inside a class declaration declare things in the order:
>      1. public
>      2. protected
>      3. private
>      4. operator

uh, why are operators special ? Are you speaking of 'friend' operators ?
Then they should be part of the public section for obvious reasons.

Other issue: each section (public, protected, private) should start with
type declarations (if any), followed by methods, (static, then 
ordinary), followed by member variables (static, then ordinary).

>    Justification: The more likely people are to need something, the
>     closer it should be to the top of the list.
> 
>  - Class access labels (private, public, etc.) should be indented 2
>     spaces relative to the start of the class.  Eg:
>      class Foo {

class Foo
{


>        public:
>          Foo();
>          ~Foo();
>        private:
>          ...
>      };
> 
>    Justification: Members are logically contained by the class
>     declaration, so should be indented 4 spaces relative to that.
>     Access labels should be indented intermediate between the class
>     and the members.  Hence, 2 spaces.
> 
> 
> Naming
> ------
>  - Naming:
>     Name classes and namespace portions LikeThis (StudlyCaps, with the
>      first letter capitalized). Eg:
>       class FooBar {};
>       namespace FooBar::BazQuux {};

no semicolon after namespace definition ! (The semicolon after the class 
definition is needed since C++ allows a class definition to be written 
with a variable declaration in the same declaration, i.e.

class Foo {} foo;

so the semicolon is needed to disambiguate. Not so with namespaces.


>     Name everything else like_this, with underscores separating words.
>      Eg:
>       int FooBar::baz_quux() {};
>       static int foo_bar;
>     Constant variables and macros should be capitalized, LIKE_THIS.

macros should be entirely banned.


>      This includes enum values. Eg:
>       static const int FOO_BAR = 1;
>     Private variables should have "my_" prepended to them.  Private
>      methods should have "_" prepended to them.
> 
>    Justification: It's much more readable than theStupidJavaWay, etc.
>     See http://www.berlin-consortium.org/review.html for justification
>     on the my_ convention, and more notes on meaningful naming.
>     [[FIXME: should be merged in here at some point.]]

uh, well, I'v been using the '_' prefix for member variables all over 
the place. I don't like the idea to have to change that. I wouldn't 
decorate private methods any special.

> Actual code
> -----------
>  - Do not use macros, unless you have permission, in writing, in
>     triplicate, from Stefan.

hehe, I like that rule :-)

>    Justification: Macros are weird, wild, and generally have better
>     replacements in C++.  Stefan is a member of the Holy Order of Foo,
>     an organization dedicated to the everlasting preservation of the
>     safety of defenseless types.  It works out.

yes it does.

> Commenting
> ----------
> This is not a comprehensive list; comment your code whenever you think
> it needs it, or whenever you think your read needs it.  But there are
> a few things worth special note.
>  - Put Synopsis comments on everything!  [[FIXME: are there any
>     docs anywhere on how to format Synopsis comments?  Does
>     Synopsis support @return and the like?  This section should be
>     fleshed in with a description of how to actually write these
>     comments... or a link to such documentation, if it exists.]]

right, something to be worked on.


>  - Put a comment starting with "FIXME:" if you are using a weak way to
>     do something, like not checking for the existance of files, code
>     that breaks in cornercases, etc.
> 
>     Do try *not* to put FIXME:'s into your code, Do try to remove
>     other people's FIXME:'s.

[1]


>     Mark code that is used to work around a compiler limitation (like
>     functionality that is given in the C++ standard to be there but is
>     missing from your compiler with a comment starting with
>     WORKAROUND: if this workaround is so ugly that it should be fixed
>     once a compliant version of your compiler gets out.
> 
>    Justification: later grepping, etc.  Someone should create a
>     synopsis plugin that scans the source for these sorts of comments,
>     puts them on a web-page somewhere, with context and links to the
>     actual source...

we should have similar rules for deprecated code (this applies mostly to
interface level stuff), i.e. we should even think about something that 
is synopsis-parsable, so people can look up whether some part of an API
is going to be removed soon, etc.


[1] We should discuss how to modify existing code, i.e. who is entitled
     to modify what. It's more a question of politeness than anything
     else, but I'd much prefer people to at least try to contact the
     original author of a piece of code instead of messing themselfs with
     code they possibly don't fully understan.
     There have been some cases where someone modified my code in a way
     that it didn't work any more, or at least it wasn't consistent with
     my original intention. And even if it works, I think it's just a
     matter of respect to ask the author (if ever he is still around) to
     incorporate a modification (or ask for his agreement), instead of
     just doing it.

     But that is something everybody has to judge for himself...


Nathaniel, that's an extremely useful document. Thanks a lot for your work !

Stefan