[[Include(Templ/PageHeader)]]
= OpenSG Styleguide =
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.
||[[PageOutline(2-3,,inline)]]||
== Code Style ==
As portability is an important goal of the project, the code has to compile on Windows and Unix.
So do not write code that depends on platform-specific compiler features. If it's not avoidable, bracket them with an appropriate `#ifdef`. The following preprocessor constants should be used for the given platforms:
* Linux: `__linux`
* Windows: `WIN32`
* MacOSX: `__APPLE__`
* Solaris: `__sun`
* IRIX: `__sgi`
* HP-UX: `__hpux`
== 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.
{{{
#!cpp
class Light;
class DirectedLight;
}}}
Methods should use the verb[adjective]noun convention.
{{{
#!cpp
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 :
{{{
#!cpp
_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 our 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 they are not wanted, they are placed within the private section and preceded by the following comment :
{{{
#!cpp
/*!\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.
=== Functors ===
To enhance the portability : the template arguments for a functor create function ''must'' be provided
on usage, see example below.
{{{
#!cpp
VRMLWriteAction::registerLeaveDefault(
MaterialGroup::getClassType(),
osgTypedFunctionFunctor2CPtrRef<
Action::ResultE,
CNodePtr ,
Action *>(&VRMLWriteAction::writeMatGroupLeave));
}}}
=== STL ===
Pointers are '''''NOT''''' iterators, and iterators are '''''NOT''''' pointers, even if some of them support the same concepts.
=== 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 style.
Maximum 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.
{{{
#!cpp
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.
{{{
#!cpp
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 (see below). Try it before rejecting it. Opening and closing braces should really be in the same column (ANSI style).
{{{
#!cpp
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.
{{{
#!cpp
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.
{{{
#!cpp
if((something + 2) == 5)
{
if((something + 2) == 4 )
doSomethingSimple;
}
else
{
doSomethingElse;
}
}}}
There should be a space between a `,` and the next function argument
{{{
#!cpp
void foo(Int32 val1, Int32 val2);
void foo(Int32 val1, Int32 val2)
{
bar(val1, val2);
}
}}}
Functions having no argument should be written as
{{{
#!cpp
void foo(void);
}}}
not
{{{
#!cpp
void foo();
}}}
There should be no space between `&` or `*` and the variable name
{{{
#!cpp
void foo(Int32 *iP, Int32 &iR);
}}}
=== Switch/Case ===
They follow the general rules about bracketing and indentation. The goal is to try to keep the case labels visible, to see what's going on, by leaving an empty line before them.
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.
{{{
#!cpp
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;
}
}}}
For very long class/method names line breaks need to be used judiciously...
{{{
#!cpp
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:
{{{
#!cpp
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 :
{{{
#!cpp
class Foo
{
private:
struct FooHelper;
friend struct FooHelper;
struct BarHelperForFooHelper
{
};
struct FooHelper
{
BarHelperForFooHelper _val1;
UInt32 _val2;
};
};
}}}
=== Macros ===
The general advice to not use macros also applies to OpenSG, but there are some areas where their use seems justified. Most of these cases currently simplify the generation of boilerplate or highly repetitive code fragments. These macros usually are not uglified nor prefixed with `OSG_`, so it is important to `#undef` them right after their point of use.
== Commenting/Documentation Rules ==
We use [http://www.stack.nl/~dimitri/doxygen/ doxygen] for all code commenting.
The main philosophy behind our commenting is: `comment as much as needed, but keep the headers clean`
We use the QT comment style (/*!) is used, because it allows separating the brief from the full comment.
''Classes''
* Only the brief comment should be in the header
* The full comment should be in the source code (.inl or .cpp)
* Add classes to their respective group(s). Groups are an important aid for useful structuring, use them.
Example:
{{{
#!cpp
/*! \class osg::Node
\ingroup GrpSystemFieldContainer
\brief Nodes form the backbone of the scenegraph.
*/
class OSG_SYSTEMLIB_DLLMAPPING Node : public AttachmentContainer
{
}}}
''Parts''
The `public`, `protected` and `private` parts should be marked with a line like this:
{{{
#!cpp
/*========================== PUBLIC =================================*/
}}}
before the keyword.
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 the methods.
Example:
{{{
#!cpp
/*---------------------------------------------------------------------*/
/*! \name Destructors */
/*! \{ */
virtual ~BoxVolume(void);
/*! \} */
}}}
* Put detailed documentation in the '.cpp' file.
* If the method is reimplemented from another class, only the original class's version should have documentation (doxygen will take care of repeating the documentation form the original class) unless something has changed in the behavior for this implementation. If you want to expand the original class's documentation, you can use `\copydoc` to get the original documentation.
* For pure virtual methods you can use an explicit '\fn' section in the .inl or .cpp, if it exists.
* Please use the `\param` structured commands to give a quick explanation of the parameter's use. They should mention the legal ranges/values of the parameter. If sensible, reference the documentation pages for general concepts from the functions/methods in a see also (`\sa`) section.
* Use the `\return` command to give details of the return value for the method.
* If sensible, do associate free-standing functions with classes they're related to (`\relates`).
* You have to give warnings (`\warning`) for non-intuitive results or side effects.
''Code''
The ordering in the `.cpp` files should be:
* full class documentation
* class member variables documentation
* class typdef/enum documentation
* includes (''Why aren't these first in the .cpp file like normal?'')
* group definition(s) if needed
* 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.
Do use `\warning` to talk about unintuitive side-effects or constraints of a method. Use `\post` commands to mention non-obvious postconditions (i.e. effects on objects other than the one a method is called on and the parameters).
''Concepts''
Longer explanations about concepts and connections between parts of the system fit better into a more textual environment like a Wiki. The [wiki:Doc/DevGuideFull] is a collector for these kinds of pages. Please add a new page for new concepts in the !DevGuide/ directory and add your new page to the index on [wiki:Doc/DevGuideFull]. It uses the !WikiInclude macro to merge all the different pages into one big document that is suitable for printing.
To connect from the doxyxgen documentation to these and other pages on the Wiki we have two aliases in doxygen. \wiki(page) links to a page on the Wiki, \guide(page) links to a page in the dev guide (i.e. inside !DevGuide/). Even though you can get to the !DevGuide pages using \wiki, please use \guide in case we need to restructure things later.
To link from the !DevGuide pages to the doxygen docuentation we use the [http://trac-hacks.org/wiki/DoxygenPlugin DoxygenPlugin for Trac]. We have extended it to support smarter and more extensive linking to doxygen objects. You can link to classes, structs and methods directly from a doxygen: link. Some examples:
* [doxygen:osg::VectorInterface] Link to class with namespace
* [doxygen:VectorInterface] Link to class (osg:: is the default namespace and doesn't have to specified)
* [doxygen:TypeTraits] Link to templated class
* [doxygen:VectorInterface::isZero] Link to member method
* [doxygen:VectorInterface::operator+] Link to operator
* [doxygen:VectorInterface::getValues] Link to first version of overloaded method
* [doxygen:VectorInterface::getValues(void)] Explicit link to first version of overloaded method
* [doxygen:"VectorInterface::getValues(void) const"] Explicit link to second version of overloaded method
== Test Style ==
For unit testing we use UnitTest++, see: [http://unittest-cpp.sourceforge.net/UnitTest++.html UnitTest++ Documentation page].
The naming scheme for unit tests is `OSGTest.cpp`, where `` is the name of the source file the test refers to. If there is not a unique file or class tested by a single test, the most appropriate should be picked.
The test files themselves should start with the copyright notice, followed by `#include ` and the needed OpenSG includes. To avoid name clashes every test should enclose its contents in a `SUITE(Test)` definition. Inside the `SUITE` you can have as many `TEST`s as you like, but it is desirable to keep them small and named sensible to improve diagnostics. The code oviously has to follow the above style guide. You can use the [attachment:OSGTemplateTest.cpp template file] as a starting point for new tests.
One *important* point to remember is that you may not call `osgInit` inside a test, as this is done by the testing harness upon startup for you.
Need more here.
== Discussion ==
>> AB: What about pure virtual methods?
>
> ''?? They are documented in the .cpp file, via a \fn -- or do you mean something else ? (cneumann)''
Yup. DR
> Is it really nessecary to state that a function doesn't take a value? I would opt for `virtual ~BoxVolume();` as being more readable (jspringer)
Hm, I don't know. I'd like to make it explicit. DR.
>> "We use the QT comment style (/*!) is used, because it allows separating the brief from the full comment."
>I don't see this anywhere in the doxygen docs. The only thing with javadoc and brief is if you turn on the automatica brief documentation to take the first line of the comment as a brief (which is actually helpful IMHO, but I digress). So why can't we use javadoc style? ie. /** */
> Any reason for Qt comment style vs. javadoc? -AB
> Why does the example above use a "\class" command instead of just letting doxygen figure it out? -AB
I've had bad experience without them, but I'd be willing to give it another try. AFAIR the \group ones don't work if you don't give a \class. DR
>> Can we please, please, please put the documentation for member variables and typedefs in the header file with the thing we are documenting. As I have argued before I think this is much better because it keeps the documentation close to the code. It is then easy to update. I think that should be another philosophy, "Put the documentation near the thing you are documenting and keep it up to date whenever you change something." -AB
>I heartily object to documenting methods in the headers, I'd be ok with member vars and typedefs, I'm uncertain about enums. Comments? DR
I would say document everything where it is defined. So vars, typedefs, and enums would be in the headers. Methods in the .cpp or .inl files. -AB
--[[BR]]
Do we want `\param[in]`, `\param[out]` to be used for parameters ? I would say yes, comments ? (cneumann)
I would say yes too, especially since we use references quite a lot, and mostly for input. In most places they're const anyway, but just to make sure it's clear the in and out would be useful. ''--DR''