Pegasus Enhancement Proposal (PEP)

PEP #: 221

Title: Pegasus Coding Conventions

Status: Approved

Version History:

Version Date Author Change Description
1.0 23 February 2005 Roger Kumpf Initial submittal, based on documents in pegasus/doc directory in CVS (conventions.txt, portability.txt, CodeReviewNotes.txt, and DOCREMARKS).
1.1 1 March 2005 Roger Kumpf Updated based on 3/1/05 architecture review comments.
Approved version (Ballot 95).
2.0 21 March 2005 Roger Kumpf Opened up for additional conventions.
2.1 5 August 2005 Roger Kumpf Updated based on comments and architecture team discussions.
2.2 21 February 2006 Roger Kumpf Updated based on review comments.
2.3 24 February 2006 Roger Kumpf Updated based on review comments and architecture team discussion.
2.4 10 March 2006 Roger Kumpf Updated based on ballot comments.
Approved version (Ballot 111).
3.0 10 March 2006 Roger Kumpf Opened up for additional conventions.
3.1 20 November 2008 Roger Kumpf Updated based on review comments.
3.2 26 November 2008 Roger Kumpf Updated based on review comments.
3.3 4 December 2008 Roger Kumpf Updated based on review comments.
Approved version (Ballot 160).
3.4 22 January 2013 Marek Szermutzky Adding section on Unused's (variables, arguments, values etc.)
3.5 22 January 2013 Marek Szermutzky macro PEGASUS_FCT_EXECUTE_AND_ASSERT added
3.6
19 November 2013
Karl Schopmeyer
Add rule about defaults in switch constructs (see Formatting, 22)

 


Abstract: This document formalizes the Pegasus coding conventions.


Definition of the Problem

Proposed Solution

The following items comprise the Pegasus coding conventions.

Formatting

  1. Indent by increments of four spaces. Indent comments equally with the associated code.
  2. Do not use tab characters.
  3. Lines must not span more than 80 columns.
  4. Remove trailing spaces at the end of a line. (Note: This also applies to Makefiles, where trailing spaces may cause unintended whitespace in generated files.)
  5. Put opening brace on a line by itself, aligned with control keyword or method signature. Do this:
        for (...)
    {
    }
    Not this:
        for (...) {
    }
    Or this:
        for (...)
    {
    }
  6. Use braces around the body of a control block (e.g., if, for, or while statement), even if the body contains just a single statement. Do this:
        for (...)
    {
    i++;
    }
    Not this:
        for (...)
    i++;
  7. Use a new line for else statements rather than placing then on the same line as a preceding '}'. Do this:
        if (...)
    {
    i++;
    }
    else
    {
    j++;
    }
    Not this:
        if (...)
    {
    i++;
    } else
    {
    j++;
    }
  8. Use normal indenting for parameters in a complex method signature:
        void MyClass::myMethod(
    const char* someReallyLongName,
    const char* someOtherReallyLongName);
  9. Use normal indenting for all arguments in a complex function call. Do this:
        callingMyFunction(
    arg1,
    arg2,
    arg3);
    Not this:
        callingMyFunction(arg1,
    arg2,
    arg3);
    Each argument should be placed on a separate line. An exception is made for PEG_TRACE and PEG_TRACE_CSTRING statements, where the first two arguments should be placed on the opening line:
        PEG_TRACE((TRC_HTTP, Tracer::LEVEL3,
    "Connection IP address = %s",
    (const char*)_ipAddress.getCString()));
  10. For an if, while, or for condition that does not fit on a single line, indent peer subconditions according to their level of nesting and indent other continuation lines using normal indenting. Do this:
        if ((variable1 == variable2) ||
    ((anotherVariableToConsider ==
    variable1 + variable2) &&
    (dayOfWeek == "Tuesday")))
    Not this:
        if ((variable1 == variable2) ||
    ((anotherVariableToConsider ==
    variable1 + variable2) &&
    (dayOfWeek == "Tuesday")))
  11. For a statement that does not fit on a single line, all continuation lines should be indented once from the first line. For example:
        sum = a + b + c +
    d + e + f +
    g + h;
  12. Do not separate a return type onto its own line. Avoid this:
        int
    f()
    {
    }
  13. Avoid use of "(void)" for an empty parameter list. Use "()" instead.
  14. Avoid adding a space between function/method name the opening parenthesis. Do this:
        void f(int i);
    f(10);
    Not this:
        void f (int i);
    f (10);
  15. Include a space between a keyword and an opening parenthesis. Do this:
        if (i > 0)
    for (i = 0; i < 10; i++)
    Not this:
        if(i > 0)
    for(i = 0; i < 10; i++)
  16. Do not add spaces around a condition. Do this:
        if (cond)
    while (cond)
    Not this:
        if ( cond )
    while ( cond )
  17. Do not add a space between a template name and its type. Do this:
        Array<Uint8> myArray;
    Not this:
        Array <Uint8> myArray;
  18. Avoid aligning variable names in a declaration list. Do this:
        int x;
    float y;
    Not this:
        int      x;
    float y;
  19. Avoid indenting "public:", "protected:", and "private:". Do this:
        class X
    {
    public:
    int f();
    ...
    private:
    int _g();
    ...
    };
  20. The '#' indicating a preprocessing directive is placed at the beginning of a line. Nested preprocessing directives are indented with one space after the '#' for each level of nesting. The #ifndef used for header file include protection is not considered a level of nesting. For example:
    #ifdef PEGASUS_HAS_SIGNALS
    # ifdef PEGASUS_OS_TYPE_WINDOWS
    # include <windows.h>
    # else
    # include <unistd.h>
    # endif
    #endif
  21. In a switch statement, indent the case statement from the switch and indent the case logic from the case statement. For example:
        switch(n)
    {
    case 0:
    printf("Rock");
    break;
    case 1:
    printf("Paper");
    break;
    case 2:
    printf("Scissors");
    break;
    default:
    printf("Error");
    break;
    }
  22. A switch construct for an enumerated type the construct MUST contain either all of the possible cases  or the default statement but not both. We encourage including all of the enumerated types since this provides a compile time check if the enumerated type changes.  Do not include the default statement if all of the possible enumerated types are included in the case statements. At least in gcc this is enforced in the standard build with the -Werror=switch flag which generates an error statement when the switch has an index of enumerated type and lacks a case for one or more of the named codes of that enumeration.
  23. Do not put parentheses around the return value in a return statement.

Naming

  1. For class/struct names, use mixed case with initial upper case and no underscores: ThisIsAClassName.
  2. For public member/variable names, use mixed case with initial lower case and no underscores: thisIsAMemberName.
  3. For public method/function names, use mixed case with initial lower case and no underscores: thisIsAMethodName().
  4. Prepend an underscore to private and protected member and method names: _thisIsAPrivateMemberName, _thisIsAPrivateMethodName().
  5. General constants and macro names should begin with PEGASUS_.
  6. Use this format for constant and macro names: PEGASUS_CONSTANT_NAME.
  7. Files names should use mixed case with initial upper case and no underscores: ThisIsAFileName.cpp.
  8. A file should have the same name (and case) as the class it contains.
  9. Environment variables must begin with PEGASUS_ and have this form: PEGASUS_ENVIRONMENT_VARIABLE. This applies to environment variables that control the build and test configuration as well as those (if any) used by Pegasus at run time by forcing an error if the 
  10. Test executable names should begin with 'Test' (to distinguish them from other executables) and should use mixed case with initial upper case and no underscores. The name should clearly indicate the feature being tested. For example: TestFeature1.

Style

  1. In a header file, use angle brackets (with a fully qualified path) rather than quotes when including a file. Do this:
        #include <Pegasus/Common/Array.h>
    Not this:
        #include "Array.h"
  2. Use "0" rather than "NULL" as a null pointer value.
  3. Avoid throw() declarations. These may be used only when a method must not (and will not) throw any exception not specified in the throw clause. When a throw() clause is needed, include it on the method declaration and definition.
  4. Use const declarations liberally, but not on "plain old data type" parameters.
  5. Catch exceptions by const reference when feasible.
  6. Avoid inlining of large functions.
  7. Avoid committing binary files to CVS.
  8. Resolve compile warnings.
  9. Define variables as late as possible. A single object initialization is more efficient than initialization followed by assignment. For example, this logic involves just a single String construction:
        String s = foo();
    while this logic causes two String constructions and an assignment:
        String s;
    ...
    s = foo();
  10. Present resources as part of classes, do not use them directly.
  11. Use new and delete to manage dynamic memory, not malloc, free, or strdup.
  12. When rethrowing an exception from a catch block, avoid specifying the object to throw so that exception subtype information is not lost and construction of an extra exception object is avoided. Do this:
        catch (Exception&)
    {
    ...
    throw;
    }
    Not this:
        catch (Exception& e)
    {
    ...
    throw e;
    }
  13. Do not check whether a pointer is non-null before deleting it. The check is unnecessary, since it is also performed by the delete operator. Do this:
        delete ptr;
    Not this:
        if (ptr)
    {
    delete ptr;
    }
  14. Avoid using the compiler default implementations for default constructors, copy constructors, and assignment operators. These should be declared as private and left unimplemented if they are not intended to be used. A decision to use a compiler default should be documented with a comment in the class declaration.
  15. Avoid the use of optional parameters in methods which have interface compatibility requirements.
  16. Avoid reading variables from the environment in production (not debug or test) runtime code.
  17. Definitions for the external interface declarations should not appear in the external interface header files. Implementation logic for the external interfaces should be contained in the associated source files.
  18. Pass object parameters by reference, when feasible, to avoid unnecessary copy constructions. For example, this:
        void func(const String& s);
    is preferred to this:
        void func(String s);
  19. Avoid using the C++ standard template library (STL) and the standard string class. (These tend to produce bloat and may have portability issues.) These files should not be included: <string>, <vector>, <map>, <mmap>, <set>, <mset>, <stack>, <queue>, and other STL header files.
  20. Do not call assert() directly. Instead, use PEGASUS_ASSERT() in production code and PEGASUS_TEST_ASSERT() in test programs.
  21. Do not explicitly initialize Pegasus Strings to String::EMPTY. This is the default value.
  22. Do not add a null terminator to Pegasus Buffer strings. Buffer::getData() ensures the resulting string is null-terminated.

Documentation

  1. Class members and methods should be documented with DOC++ comments.
  2. Use /** */ rather than /// for DOC++ comments.
  3. Use this comment style for DOC++ comments:
        class X
    {
    public:

    /**
    Creates widgets.
    @param numWidgets The number of widgets to create.
    @return true if successful, false otherwise.
    */
    Boolean createWidgets(Uint32 numWidgets);
    };
  4. A description should end with a period, even if it is brief:
        /**
    Does something useful.
    */
  5. Add a blank line between documented methods:
        /**
    Does something quite useful.
    */
    void f();

    /**
    The previous method has a newline after it
    to improve its visibility.
    */
    void g();

Portability Considerations

  1. The use of platform specific #ifdef's should be minimized and discouraged. It is preferred to abstract the mechanics of dissimilar platforms in separate modules (such as DirPOSIX.cpp vs. DirWindows.cpp) or by using abstraction functions or macros such as those that appear below.
  2. In header files, identifiers from the standard library (such as ostream, istream, cout, and cerr) must be enclosed in a PEGASUS_STD() macro (which prepends std:: to the argument on some platforms).
  3. Do not use the PEGASUS_STD() macro in a source file. Instead, specify PEGASUS_USING_STD; at the beginning of the file.
  4. Do not use PEGASUS_USING_STD; or PEGASUS_USING_PEGASUS; in a header file.
  5. Avoid use of conditional compilation for obscuring platform differences. Use platform abstractions in the appropriate platform files or in the System*.cpp files in Common.
  6. Windows requires symbols to be explicitly imported/exported from dynamic libraries. Linkage.h files are used to define the necessary linkage macros. Each dynamic library that exports symbols should define a PEGASUS_<LIBRARY>_LINKAGE symbol in a Linkage.h file. Each symbol that is exported from the library must be declared with this linkage macro. For example:
        class PEGASUS_COMMON_LINKAGE String;

    PEGASUS_COMMON_LINKAGE void globalFunction();
  7. A main() function must return an int (required by Windows NT).
  8. Do not use ultostr(); use sprintf() instead.
  9. Do not declare a throw clause without parentheses:
        void f() throw TooBad;
  10. Do not include a definition of a static member with its declaration:
        class X
    {
    public:

    static const Uint32 COLOR = 225;
    };
    Use this instead:
        class X
    {
    public:

    static const Uint32 COLOR;
    };
    And add the definition in the source file:
        const Uint32 X::COLOR = 225;
  11. Use PEGASUS_64BIT_CONVERSION_WIDTH for printf and scanf conversions of 64-bit integers rather than ll or L. Do this:
        Sint64 i64 = 10;
    printf("i64 value = %" PEGASUS_64BIT_CONVERSION_WIDTH "d.\n", i64);
    Instead of this:
        Sint64 i64 = 10;
    printf("i64 value = %lld.\n", i64);
  12. Do not include class scoping on methods or members in a class definition. Do this:
        class X
    {
    public:

    int myMethod();
    };
    Not this:
        class X
    {
    public:

    int X::myMethod();
    };
  13. Do not use Pegasus:: to scope symbols. Not all platforms support C++ namespaces, and this will cause a compilation failure. If a symbol must be explicitly scoped to the Pegasus namespace, use the PEGASUS_NAMESPACE() macro instead.
  14. Use consistent declarations of const parameters in method declarations and definitions.

Avoiding Unused parameter, variables, values, functions ...

In general, avoid unused variables, parameters, values and functions. Many compiler come with support to automatically detect these, use them. The OpenPegasus build using gcc will flag unused variables as errors. The Pegasus Architecture team may completely disallow checkin of code that generates unused* warnings to CVS in the future.

  1. Avoid unused parameter warnings. In case the function signature cannot be changed since the function codes an interface, leave the parameter name off in the function definition while keeping the parameter type in place. In the following example of function main() the parameter argv is used later on but parameter argc is not. Do this:
    int main(int, void** argv)
    {
    }
    Not this:
    int main(int argc, void** argv)
    {
    }
  2. Avoid unused variables when the return value of a function is assigned to a variable only to be used to do an assert check with PEGASUS_ASSERT by instead using macro PEGASUS_FCT_EXECUTE_AND_ASSERT. It helps avoid such unused variables when PEGASUS_NOASSERTS is enabled (assertion disabled). PEGASUS_FCT_EXECUTE_AND_ASSERT compares the return value of function against VALUE for equalness but only if asserts are enabled. The Function FCT will always be called (equal if asserts enabled or disabled). Do this:
    PEGASUS_FCT_EXECUTE_AND_ASSERT(true, f());
    Not this:
    bool returnCode = f();
    PEGASUS_ASSERT(true == returnCode);

Discussion

Suggestions for additional conventions are listed here for discussion:


Copyright (c) 2005 EMC Corporation; Hewlett-Packard Development Company, L.P.; IBM Corp.; The Open Group; VERITAS Software Corporation

Permission is hereby granted, free of charge, to any person obtaining a copy  of this software and associated documentation files (the "Software"), to deal in the Software without restriction, including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions:

THE ABOVE COPYRIGHT NOTICE AND THIS PERMISSION NOTICE SHALL BE INCLUDED IN ALL COPIES OR SUBSTANTIAL PORTIONS OF THE SOFTWARE. THE SOFTWARE IS PROVIDED  "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.


Template last modified: March 9th 2004 by Martin Kirk
Template version: 1.8