TWiki . Main . StyleGuide	TWiki webs: Main | TWiki | Test
	Main . { Main . { Home | Users | Changes | Index | Search | Go } }

To improve code readibility and maintainability there should be a common style guide for the OpenSG codebase. We did define some rules at the beginning, but the were not enforced and fell by the wayside a little. Thus this attempt to write them down again and start enforcing them.

• Code Style
• Source Style
• Test Style

Code Style

As portability is an important goal of the project, the code has to compile on Windows and Unix. At the same time far-reaching appeal is important.

The combination means one bad thing: Microsoft Visual C++.

It absolutely has to be possible to write applications using MSVC++ out of the box. As a lot of the intelligence of the system is in the headers that's a problem. Thus there is probably no way around having to support the M$ compiler directly. :(

Source Style

Note: None of these rules are cast in stone, they can always be bent if the need arises. Just be prepared to give us some good arguments why you didn't follow them. ;) Some of them are softer than others, though.

Naming

All symbols are part of the osg namespace to prevent collisions with other libraries, especially for the simple types.

var names != type names, this is a must

Constants should be named using incaps notation and starting with an uppercase letter (e.g. "MyConstant"), this holds for enum values too. This reduces the danger of conflicts with #defines in headers included from anywhere (like "CONTINUE" is #defined under windows, "Continue" is not)

Whoever uses near, far, min, max, errno or the like as a variable or type name will be punished by C++ forbiddance and forced to use java for at least four weeks

Class names should be nouns. Basic classes should use simple nouns, related classes the name of their base class plus their own name.

class Light; 
class DirectedLight;

Methods should use the verb[adjective]noun convention.

Light::getColor(); 
Material::getSpecularColor();

Include Order

You must not include GL/gl.h, GL/glu.h and GL/glut.h directly as there are platforms which simply do not provide the GL subdirectory (e.g. darwin). Use OSGGL.h, OSGGLU.h and OSGGLUT.h instead.

C Function Names to be used by dlsym

C function names to be used by dlsym must be preceeded by OSG_DLSYM_UNDERSCORE as some platforms mangle for example foo to _foo instead of foo. So the way to register extension functions is :

    _extTex3D = Window::registerExtension(OSG_DLSYM_UNDERSCORE"GL_EXT_texture3D");

OSGBaseFunctions

The main purpose of these functions is to hide plattform dependencies. They should be used instead of the native functions provided by some

plattforms as they make my (GV) life porting this stuff a little bit easier.

Constructors and Nonstatic Data Member Initialization

Every nonstatic data member of a class must be initialized through the mem-initializer-list before the function body of the constructor is executed, in the order they appear in the class definition.

Copy Constructor and Copy Assignment Operator

Every class must declare its copy constructor and copy assignment operator. If the definition is ommited they are placed within the private section an preceded by the following comment :

    /*!\brief prohibit default function (move to 'public' if needed) */
    CLASSNAME(const CLASSNAME &source);
    /*!\brief prohibit default function (move to 'public' if needed) */
    void operator =(const CLASSNAME &source);

Inlines

Don't put inline method code into the .h, put it into .inl instead. You should declare the methods that you're going to inline inline, otherwise some compilers complain.

 I do not see why you have to declare them inline, defining them inline should be enough (GV)

Functors

To enhance the portability : the template arguments for a functor create function must be provided on usage, see example below.

    VRMLWriteAction::registerLeaveDefault(
        MaterialGroup::getClassType(), 
        osgTypedFunctionFunctor2CPtrRef<
            Action::ResultE,
            CNodePtr        ,
            Action         *>(&VRMLWriteAction::writeMatGroupLeave));

STL

Pointers are NO iterators, and iterators are NO pointers, even if some of them support the same concept.

Ordering

Members and methods in classes should be in the order public - protected - private, to show people looking at the sources the stuff they can actually use before the internals that are not accessible from the outside.

The functions inside the larger sections should be ordered as follows:

• constructors
• destructors
• get methods
• set methods
• class specific methods (these will be different for every class)
• class specific operators
• comparison operators
• assign operator

Sometimes a short private section is needed before everything else, e.g. for defining some internal types that are used for constants or enums. It's ok to do that, just keep it as short as possible.

Indents and Formatting

This is the area where most religious struggles happen... This is our suggestion.

Line length is 79 characters. No line should have more than that.

Indents are 4 spaces, no tabs should be used for indenting, as they will always confuse some people.

We are not amused by setting block opening brackets ({) the K&R way. Block opening brackets should be placed into the next line.

class Foo
{
};

void bar(void)
{
    if(fooBar == true)
    {
        ...
    }


    while(fooBar == false)
    {
        ...

    }
};

There is no space between opening parentheses and the next argument, neither between the last argument and the closing parentheses. There are spaces between arguments, though.

a = (1 + 2) * (3 + 4);

There are no spaces between if and the parenthesis, otherwise conditionals are just like expressions. This looks quite ok with syntax highlighting, which TWiki doesn't do. Try it before rejecting it. Opening and closing braces should really be in the same column (ANSI style).

if((something + 2) == 5)
{
    doSomething;
}

If the if clause consists of a single, simple expression it can be written in the next line without braces.

if((something + 2) == 5)
    something = something * 3 + 2;

This only applies to simple expressions. if is not a simple expression, and it does not apply if there is an else clause, in these cases use braces.

if((something + 2) == 5)
{
    if((something + 2) == 4 )
       doSomethingSimple;
}
else
{
    doSomethingElse;
}

There should be a space between a ',' and the next function argument

void foo(Int32 val1, Int32 val2);

void foo(Int32 val1, Int32 val2)
{
    bar(val1, val2);
}

Functions having no argument should be written as

void foo(void);

void foo();

There should be no space between & or * and the variable name

void foo(Int32 *iP, Int32 &iR);

Switch/Case

They follow the general rules about bracketing and indentation. The exception is that they try to reduce the screen space use a bit by not indenting the case labels.

The goal is to try to keep the case labels visible, to see what's going on.

switch(hugo)
{
case HUGO0:     doSomethingCool();
                break;

case HUGO1:     {
                UInt32 tempint = getTempInt();
                doSomethingCooler(tempint);
                }
                break;

case HUGO2:     veryShort();   break;
case HUGO3:     veryShort2();  break;

default:        nothingCoolToDo();
                break;
}

This doesn't always work, especially if the label constants or code lines are long. In that case indent only once, but keep the cases on separated lines.

switch(hugo)
{
case MyClass::HelperClass::HUGO0:

    doSomethingVeryCoolWithUnsuspectingClass(this->getUnsuspectingClass());
    break;

case MyClass::HelperClass::HUGO1:

    {
    UnsuspectingClass & victim = this->getUnsuspectingClass();
    suspectClassInstance->makeInconspicious(victim, MyClass::HelperClass::NOW);
    }
    break;
 
default:

    break;
}

If you need local variables, those cases have to be enclosed by {}, but don't do it for every case, it confuses more than it helps.

I'm not really happy with this yet, but don't know a better way to keep it easy to read and friendly to screen space at the same time. (DR)

I do not like it at all ;-( (GV)

I would go for this :

switch(hugo)
{
    case HUGO0:     
        doSomethingCool();
        break;

    case HUGO1:     
    {
        UInt32 tempint = getTempInt();
        doSomethingCooler(tempint);
    }
    break;

    case HUGO2:     
        veryShort();   
        break;

    case HUGO3:     
        veryShort2();  
        break;

    default:        
        nothingCoolToDo();
        break;
}

typedef MyClass::HelperClass HelperClass;

switch(hugo)
{
    case HelperClass::HUGO0:
    {
        doSomethingVeryCoolWithUnsuspectingClass(
            this->getUnsuspectingClass());
    }
    break;

    case HelperClass::HUGO1:
    {
        UnsuspectingClass & victim = this->getUnsuspectingClass();
        suspectClassInstance->makeInconspicious(victim, 
                                                HelperClass::NOW);
    }
    break;
 
    default:
    break;
}

Namespaces

Within OpenSG the std namespace is not enabled by default, the std:: prefix must be used to fully qualify elements of the standard namespace .

Standard exception classes (e.g. exception) MUST be qualified using OSG_STDEXCEPTION_NAMESPACE:: (OSG_STDEXCEPTION_NAMESPACE::exception) NOT std:: (std::exception)

Similar extensions to the STL (e.g. hash_map) MUST be qualified using OSG_STDEXTENSION_NAMESPACE:: (OSG_STDEXTENSION_NAMESPACE::hash_map) NOT std:: (std::hash_map)

Helper Classes

Helper classes containing only publicly accessible elements should be defined as structs.

Example:

class Foo
{
  public:

    struct FooHelper
    {
        UInt32 _val1;
        UInt32 _val2;
    };
};

Helper classes using protected or private elements of the enclosing class (including other nested classes) must be preceeded by their friend declaration in the following way :

class Foo

{
  private:

    struct FooHelper;
    friend struct FooHelper;

    struct BarHelperForFooHelper
    {
    };

    struct FooHelper
    {
        BarHelperForFooHelper _val1;
        UInt32                _val2;
    };
};

Commenting

The main philosophy behind our commenting is: comment as much as needed, but keep the headers clean. Doxygen is used for documentation generation, and the QT comment style (/*!) is used, because it allows separating the brief from the full comment. See DocumentationStyle for more details.

Classes

Only the brief comment should be in the header, the full comment should be in the source. Classes should be added to their respective group(s). Groups are an important aid for useful structuring, use them.

Parts

The public, protected and private parts should be marked with a line like this:

    /*==========================  PUBLIC  =================================*/

In general comments like this can be used to differentiate different parts, e.g. forwards from the main class or classes from each other.

Methods

Method shouldn't be documented in the header, good method and parameter names should speak for themselves. Instead use doxygen \name to group then methods.

Example:

    /*---------------------------------------------------------------------*/
    /*! \name                   Destructors                                */
    /*! \{                                                                 */

    virtual ~BoxVolume(void); 

    /*! \}                                                                 */

You can (and should) give a detailed documentation in the .cpp file.

Code

The ordering in the .cpp files should be:

• includes
• group definition(s) if needed
• full class documentation
• static variables
• methods in the same order as in the header

Comment every method/function to explain what it does and what's special about it. Don't use the \brief comments, though. This keeps the member list at the top of the page short and concise. Just use free text for documentation, forget the structured comments like \param, they just make the documentation longer and only very rarely add information.

Do use \warning to talk about unintuitive side-effects or constraints of a method.

-- DirkReiners - 10 Dec 2002
-- GerritVoss - 12 Jun 2002

Test Style

That one still needs to be defined.

-- DirkReiners - 29 Oct 2001