wiki:MapGuideCodingStandards

Version 13 (modified by jbirch, 16 years ago) ( diff )

Maybe final?

MapGuide Coding Standards

This document was originally based on a document maintained by Bruce Deschant, entitled MapGuide Coding Standards and Guidelines, rev 1.6, last updated July 7, 2008.

Revision History

RevisionDateAuthorComment
1.6July 7, 2008Bruce DeschantInitial public revision

Table of Contents

  1. Introduction
  2. Consistency with .NET
  3. Class Names
  4. Class Variables
  5. Documentation Standard
    1. .NET/C++
    2. General
  6. Object Creation and Runtime Speed
  7. Threading Guidelines
  8. Source File Naming
  9. Folder Hierarchy
  10. Deleting Files and Projects
  11. General Structure of MapGuide Files
  12. IDE Settings
  13. Stylistic Guidelines
    1. General
    2. Compiler Warning
    3. Strings
      1. Internally
      2. Across Wire
      3. Repository
      4. API Boundary
    4. Break and Goto
    5. If, Else-If, Else Statements
    6. For Statements
    7. ForEach Statements
    8. While Statements
    9. Do-While Statements
    10. Switch Statements
    11. Try-Catch Statements
    12. Return Value
    13. Expressions With ==
  14. Pointers
    1. Normal Pointers
    2. Smart Pointers
      1. Our Code
      2. 3rd Party Library
  15. Enumerations
  16. Parentheses
  17. Assert
    1. .NET
  18. Globalization
    1. .NET
    2. C++
    3. General
  19. Special Comments

Introduction

This document describes the coding guidelines to be used for the MapGuide project. Here are a few reasons why we need coding guidelines:

  • Most of the cost of software goes to maintenance
  • The maintenance is done by many different individuals over the lifetime of a given software product and not the original developer.
  • Guidelines improve readability of the software and help make the learning curve of new developers easier.

Consistency with .NET

Parts of MapGuide will be based on Microsoft's .NET framework, and therefore we should understand and follow the standards/design guidelines recommended by Microsoft for .NET. In Visual Studio .NET, you can find these under Design Guidelines for Class Library Developers. Alternatively, you can go to the help index and navigate to: 

   Visual Studio .NET
      .NET Framework
         Reference
            Design Guidelines for Class Library Developers

The guidelines discuss topics such as naming conventions, how to decide whether something should be a property or method, proper use of enumerations, delegates, and events, and lots of other things. You should become generally familiar with these guidelines.

Class Names

All class names should be prefixed with “Mg”.

Examples: 

class MgServerSelectFeatures
class MG_SERVER_MAPPING_API MgMappingUtil
class MG_SERVER_RENDERING_API MgServerRenderingService : public MgRenderingService

Class Variables

Any variable that needs to be visible outside of a class will expose itself via accessors (either protected, public, or internal). In general we only use private member variables unless there is a very good reason for any other type. Consequently, variables may be accessed directly only inside the class where they are defined. All other access is done through the accessor. This will help to support multi-threading and distributed objects.

Documentation Standard

.NET/C++

When you define a new public class, property, or method, you must follow the XML Doc standard. Typing three slashes “/” just before a class/property/method declaration will bring up a skeleton XML Doc header in .NET, for C++ you will have to enter this. You then simply need to add information in the different fields. Note that if you fail to add the XML Doc header for .NET, the code may not compile (this is an option that we will enable for all our projects).

XML Doc Standard example: 

///<summary>
///Summary of CanIgnoreElement.
///</summary>
///<remarks>
/// 
///</remarks>
///<param name="element">
///Description of parameter element
///</param>
///<param name="sFileName">
///Description of parameter sFileName
///</param>
/// <returns></returns> 
private bool CanIgnoreElement(Element element, string sFileName)

General

It’s important to stay disciplined to avoid getting into the habit of "coding now, documenting later". Whenever you add or modify any classes/properties/methods, you must provide/update the documentation as well. The documentation you write needs to be of sufficient quality so that someone other then you can work with it. We as developers are ultimately responsible for providing the technical information that is required to fully document a class/property/method – not someone else. We’ll refer to this documentation as "seed documentation", and its content should be as close as possible to the end-user reference documentation.

Note that private variables/properties/methods should be documented as well (for code readability). In this case, however, simply use the normal C++ comment style, e.g. 

// the shoe size, including half-sizes
private float shoeSize

Object Creation and Runtime Speed

As with Java, unnecessary object creation is something you should avoid whenever possible. Just be smart about it when you code. For example, if a call to a method returns a new object and you call that method multiple times in the same context, then change your code to call this method only once and reuse the returned value. Another example: use private helper objects when it makes sense to avoid unnecessary object creation.

Threading Guidelines

It’s important to follow strict threading rules when writing new code. Some of the MapGuide code will be used in a multithreaded deployment environment. Failing to follow multi-threading guidelines will be costly in the future. There are detailed guidelines provided by Microsoft's Design Guidelines for Class Library Developers ([ms-help://MS.VSCC/MS.MSDNVS/cpgenref/html/cpconnetframeworkdesignguidelines.htm local VS link]) that should be reviewed.

Here are some points from those guidelines:

  • Avoid providing static methods that alter static state
  • Be aware of method calls in locked sections
  • Avoid the need for synchronization if possible
  • Avoid using global variables

Source File Naming

The naming of source code files will match the class names except that the “Mg” prefix will be dropped. For example, if I have a class called MgClassA then all source files for that class will have the name ClassA. If this class is defined in C#, then there is only one source file with the .cs extension. If this class is defined in C++, then there are two files: the source file with extension .cpp and the header file with extension .h. No mismatches between class names and file names are allowed!

One consequence of this rule is that you can only define one class/enumeration/struct/interface per file.

Folder Hierarchy

Just as the file name reflects the class name, the folder hierarchy will reflect the namespace or module hierarchy. If MgClassA is defined in the namespace OSGeo.MapGuide.DataAccess, then relative to the project root all source files for MgClassA will reside in the OSGeo\MapGuide\DataAccess folder. You will see this hierarchy reflected in the source control application. Again, having the hierarchies match up makes it very easy to navigate to where you want to go.

A consequence of this rule is that if you change a namespace or module then you will also have to rename or restructure the file hierarchy.

Deleting Files and Projects

Eventually everyone needs to remove files and/or projects from the main source. If you’re removing files, you update the Visual Studio solution file and if applicable the Linux build script. Having done this, you still need to remove the file / project from the source control application.

General Structure of MapGuide Files

We would like all MapGuide files to have a similar structure:

  1. Copyright info
    • every source file must have this at the top
    • the actual text in the copyright is subject to change, but that doesn’t mean we shouldn’t add it
    • whenever you create a new file, find another file with the copyright and paste that into your new file
  2. Import of .NET namespaces (If applicable)
    • ideally sorted alphabetically
  3. Import of local namespaces (If applicable)
    • ideally sorted alphabetically
  4. Namespace info (If applicable)
    • as mentioned above, the namespace matches the folder path of the source file(s)
  5. Class declaration
    • includes seed documentation
  6. Constant declarations
  7. Local variable declarations
  8. Inner classes separated by section comments (If applicable)
  9. Constructors
  10. Properties/methods

For example: 

//
//  Copyright (C) 2004-2008 by Autodesk, Inc.
//
//  This library is free software; you can redistribute it and/or
//  modify it under the terms of version 2.1 of the GNU Lesser
//  General Public License as published by the Free Software Foundation.
//
//  This library is distributed in the hope that it will be useful,
//  but WITHOUT ANY WARRANTY; without even the implied warranty of
//  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
//  Lesser General Public License for more details.
//
//  You should have received a copy of the GNU Lesser General Public
//  License along with this library; if not, write to the Free Software
//  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301  USA
//

using System;
using System.Runtime.Serialization;

namespace OSGeo.MapGuide.DataAccess.Tools
{
    /// <summary>
    /// Provides the base functionality for ...
    /// </summary>
    [Serializable]
    public class MyClass : ISerializable
    {
        //-------------------------------------------------------
        // Constants
        //-------------------------------------------------------
        Constants

        //-------------------------------------------------------
        // Variables
        //-------------------------------------------------------
        Variables

        //-------------------------------------------------------
        // Inner classes
        //-------------------------------------------------------
        Inner classes

        //-------------------------------------------------------
        // Constructors
        //-------------------------------------------------------
        Constructors

        //-------------------------------------------------------
        // Properties / Methods
        //-------------------------------------------------------
        Properties / Methods
    }
}

IDE Settings

It is recommended that you use Visual Studio .NET for software development on Windwos and it helps if some settings are the same for all users. Here are the ones you should make sure are set.

Under Tools/Options/Text Editor/All Languages/Tabs, make sure the tab size is set to 4 and that the ‘Insert spaces’ option is turned on. The use of spaces everywhere rather than a mix of tabs and spaces has the following advantages:

  1. It reduces the number of non-essential differences that show up when comparing files
    • If I have my editor settings set to use spaces and I edit an area that contains tabs, Visual Studio will sometimes replace the tabs with spaces
  2. It prevents columns from lining up evenly
    • File comparison tools may treat the width of tabs & spaces unevenly (1 tab does not exactly equal 4 spaces)
    • Viewing a source file in Notepad is even worse: a tab is the equivalent of about 8 spaces

Stylistic Guidelines

General

Use the next-line convention for curly braces: 

public void Foo()
{
}

Another coding standard for C# is to preface all references to instance variables/properties/methods with the “this” keyword. For example: 

public void Foo()
{
    // update an instance member variable
    this.index = 0;

    // call an instance member function
    this.Foo();
}

From a stylistic viewpoint, this makes it very easy to distinguish which variables are member variables and which have temporary scope. In C++ this is typically done by prefixing names of member variables using “m_” and therefore the keyword “this” is not required.

All static variables are named using Pascal casing according to the .NET standard. When referencing static constants or methods, always prefix it using the actual name of the class defining the constant or static method. For example: 

public class MgFoo1
{
    /// <summary>
    /// The max iterations to use.
    /// </summary>
    public int maxIter;

    /// <summary>
    /// Default value for max iterations.
    /// </summary>
    static public int defaultMaxIterations = 10;

    ...
}

public class MgFoo2 extends MgFoo1
{
    ...

    this.maxIter = this.defaultMaxIterations;  // wrong – not an instance variable
    this.maxIter = MgFoo2.defaultMaxIterations;  // wrong - subclass doesn't declare it
    this.maxIter = MgFoo1.defaultMaxIterations;  // correct

    ...
}

Compiler Warning

This guideline strongly encourages all compiler warnings to be treated as errors and therefore they should be fixed.

Strings

Internally

For the MapGuide project the internal string format to use will be wstring. For .NET development the String class will be used.

Across Wire

The UTF-8 standard will be used for transmitting strings across the wire between any of the MapGuide products (client, web tier, and server).

Repository

The resource repository used by MapGuide will use the UTF-8 standard for storing strings.

API Boundary

MapGuide will use the following string format for API boundaries in the web tier and the server: 

!#cpp
const wstring&

Break and Goto

Goto and break make the code very hard to read. I cannot believe that C# supports goto!!! The guideline is to avoid using break statements inside loops and to forbid the use of goto.

If, Else-If, Else Statements

The if-else statements should have the following form: 

!#cpp
if (condition) 
{
    statements;
}

if (condition) 
{
    statements;
} 
else 
{
    statements;
}

if (condition) 
{
    statements;
} 
else if (condition) 
{
    statements;
} 
else 
{
    statements;
}

For Statements

A for statement should have the following form: 

for (initialization; condition; update) 
{
    statements;
}

An empty for statement 

        for (initialization; condition; update);

Variable declaration inside for statement should NOT be done.

Don’t do this: 

for (int i=0; i<10; i++) 
{
    statements;
}

Do this instead: 

int i=0;
for (i=0; i<10; i++) 
{
    statements;
}

ForEach Statements

A foreach statement should have the following form: 

foreach (var in col) 
{
    statements;
}

While Statements

A while statement should have the following form: 

while (condition) 
{
    statements;
}

An empty while statement should have the following form: 

        while (condition);

Do-While Statements

A do-while statement should have the following form: 

do 
{
    statements;
} while (condition);

Switch Statements

A switch statement should have the following form. C# requires a terminating break or goto for each case. 

switch (condition) 
{
    case XYZ:
        statements;
        break;

    case ZYX:
        statements;
        break;

    default:
        statements;
        break;
}

Every switch statement should include a default case.

Try-Catch Statements

A try-catch statement should have the following format: 

try 
{
    statements;
} 
catch (MgExceptionClass1 e) 
{
    statements;
}
catch (MgExceptionClass2 e) 
{
    statements;
}

A try-catch statement may also be followed by finally which is always executed. 

try 
{
    statements;
} 
catch (MgExceptionClass1 e) 
{
    statements;
}
catch (MgExceptionClass2 e) 
{
    statements;
}
finally 
{
    statements;
}

Return Value

The use of a single return within a method or function is encouraged. The following structure: 

if (booleanExpression) 
{
    return true;
} 
else 
{
    return false;
}

should be: 

return booleanExpression;

Also, 

if (condition) 
{
    return x;
}
return y;

should be: 

        return (condition ? x : y);

Expressions With ==

The following order should be used because it will help catch the accidental use of "=" when "==" was intended at compile time: 

if (constant == variable) 
{
    // do something
} 
else 
{
    // do something else
}

Pointers

Normal Pointers

A pointer should always be checked before using. You should never assume that the pointer will be valid. This is a good coding style and should be done. 

if (pData != NULL)
{
        int nSize = pData->GetSize();
        ...
}

This is a bad coding style and should not be done. 

int nSize = pData->GetSize();

When you are no longer using a pointer you should set it to NULL so that it is clear that it is no longer valid. 

...
delete pData;
pData = NULL;

Smart Pointers

Our Code

The MapGuide Ptr template class acts as a smart pointer and should be used whenever possible. Example: 

MgRepository* MgServerResourceService::CreateRepository(CREFSTRING repositoryType, CREFSTRING repositoryName)
{
    Ptr<MgRepository> repository = 0;

    MG_RESOURCE_SERVICE_TRY()

    MgResourceServiceUtil::CheckArgument(!repositoryName.empty());

    if (MgRepositoryType::Library == repositoryType)
    {
        repository = new MgLibraryRepository(repositoryName, this);
    }
    else if (MgRepositoryType::Session == repositoryType)
    {
        repository = new MgSessionRepository(repositoryName, this);
    }
    else
    {
        throw new MgInvalidRepositoryTypeException(__FILE__, __LINE__);
    }

    MgResourceServiceUtil::CheckMemory(repository);

    MG_RESOURCE_SERVICE_CATCH_AND_THROW()

    assert(0 == mgException);

    (*repository).AddRef();     // Ownership is transferred to the caller

    return repository;
}

3rd Party Library

Wherever possible the standard C++ smart pointer should be used when dealing with 3rd party libraries.

The following is some sample code that uses the standard C++ smart pointer along with a typical MapGuide Exception handling mechanism: 

Bar* Foo::CreateBar()
{
    auto_ptr<Bar> bar;

    MG_TRY()

    bar = auto_ptr<Bar>(new Bar);
    assert(0 != bar.get());
    bar.DoSomething(); // might throw an exception

    MG_CATCH_AND_THROW()

    return bar.release(); // release ownership of the smart pointer
}

Standard C++ smart pointer notes:

  1. Never use auto_ptr objects as elements of STL containers because auto_ptr does not quite meet the requirements of a type you can put into containers (i.e. copies of auto_ptrs are not equivalent).
  2. Never use an array as an auto_ptr argument because auto_ptr's destructor invokes only non-array delete.

The following code is a cleaned up version of the above sample code using some helper macros. 

Bar* Foo::CreateBar()
{
    char* bar = 0;

    MG_TRY()

    bar = new char[256];
    assert(0 != bar);
    DoSomething(bar);

    MG_CATCH()

    if (0 != mgException)
    {
        delete[] bar;
        throw mgException;
    }

    return bar;
}

Note that MG_TRY(), MG_CATCH(), MG_THROW, MG_CATCH_AND_THROW and MG_CATCH_AND_RELEASE() macros can be anything specific to a MapGuide component. Example macro definitions for the resource service: 

#define MG_RESOURCE_SERVICE_TRY()                                             \
    MG_TRY()                                                                  \

#define MG_RESOURCE_SERVICE_CATCH(methodName)                                 \
    }                                                                         \
    catch (XmlException& e)                                                   \
    {                                                                         \
        MgStringCollection arguments;                                         \
        STRING message;                                                       \
                                                                              \
        if (DB_LOCK_DEADLOCK == e.getDbErrno())                               \
        {                                                                     \
            message = MgUtil::GetResourceMessage(                             \
                MgResources::ResourceService, L"MgRepositoryBusy");           \
        }                                                                     \
        else                                                                  \
        {                                                                     \
            MgUtil::MultiByteToWideChar(string(e.what()), message);           \
        }                                                                     \
                                                                              \
        arguments.Add(message);                                               \
        mgException = new MgDbXmlException(methodName, __LINE__, __WFILE__, NULL, L"MgFormatInnerExceptionMessage", &arguments); \
        (static_cast<MgThirdPartyException*>(mgException.p))->SetErrorCode(e.getDbErrno()); \
    }                                                                         \
    catch (DbException& e)                                                    \
    {                                                                         \
        MgStringCollection arguments;                                         \
        STRING message;                                                       \
                                                                              \
        if (DB_LOCK_DEADLOCK == e.get_errno())                                \
        {                                                                     \
            message = MgUtil::GetResourceMessage(                             \
                MgResources::ResourceService, L"MgRepositoryBusy");           \
        }                                                                     \
        else                                                                  \
        {                                                                     \
            MgUtil::MultiByteToWideChar(string(e.what()), message);           \
        }                                                                     \
                                                                              \
        arguments.Add(message);                                               \
        mgException = new MgDbException(methodName, __LINE__, __WFILE__, NULL, L"MgFormatInnerExceptionMessage", &arguments); \
        (static_cast<MgThirdPartyException*>(mgException.p))->SetErrorCode(e.get_errno()); \
    }                                                                         \
    catch (DWFException& e)                                                   \
    {                                                                         \
        MgStringCollection arguments;                                         \
        arguments.Add(STRING(e.message()));                                   \
        mgException = new MgDwfException(methodName, __LINE__, __WFILE__, NULL, L"MgFormatInnerExceptionMessage", &arguments); \
    }                                                                         \
    catch (const XMLException& e)                                             \
    {                                                                         \
        MgStringCollection arguments;                                         \
        arguments.Add(X2W(e.getMessage()));                                   \
        mgException = new MgXmlParserException(methodName, __LINE__, __WFILE__, NULL, L"MgFormatInnerExceptionMessage", &arguments); \
    }                                                                         \
    catch (const DOMException& e)                                             \
    {                                                                         \
        MgStringCollection arguments;                                         \
        arguments.Add(X2W(e.msg));                                            \
        mgException = new MgXmlParserException(methodName, __LINE__, __WFILE__, NULL, L"MgFormatInnerExceptionMessage", &arguments); \
                                                                              \
    MG_CATCH(methodName)                                                      \

#define MG_RESOURCE_SERVICE_THROW()                                           \
    MG_THROW()                                                                \

#define MG_RESOURCE_SERVICE_CATCH_AND_THROW(methodName)                       \
    MG_RESOURCE_SERVICE_CATCH(methodName)                                     \
                                                                              \
    MG_RESOURCE_SERVICE_THROW()                                               \

Enumerations

Please use the following format when declaring enumerations.

Example 1: 

enum MgDimensionality
{
    mdXY = 0,   /// XY
    mdXYM,              /// XY + Measure
    mdXYZ,              /// XYZ
    mdXYZM,             /// XYZ + Measure
};

Example 2: 

enum MgConnectionState
{
    mcsOpen = 0,   /// Connection is still open
    mcsClose,      /// Connection has been closed
    mcsBusy        /// Connection is busy processing other request
};

You will notice that the 1st letter of each word is part of the enumeration value. Also, when using enumerations in your code ALWAYS do the following: 

ConnectionState::mcsOpen

Do NOT do the following: 

mcsOpen

Parentheses

Even if the operator precedence seems clear to you, it is best to use parentheses to avoid any confusion by others. 

if (a == b && c == d)     // Bad
if ((a == b) && (c == d)) // Good

Assert

.NET

Use Debug.Assert ( condition ) to do assertions. Do not use Trace.Assert as that will not be removed for release build.

For C++ .NET projects you must do the following or else the Debug calls will still be called in release builds. 

#ifdef _DEBUG
Debug::WriteLine(SrsErrorMessage(status));
#endif

Globalization

.NET

All strings which need to be localized must be placed into resx resource files.

C++

All strings which need to be localized must be placed into res resource files.

General

Strings which do not need to be localized can remain in the code. However, you should indicate the string is non-localizable by adding a NOXLATE comment, e.g. 

    if (filename.EndsWith(".txt"))    // NOXLATE
    {
        ...
    }

The comment indicates that the author of the code explicitly wants to exclude the string from being localized.

Exception string messages should be globalized if possible.

Special Comments

Use BOGUS in a comment to flag something that is bogus but works. Use FIXME to flag something that is bogus and broken. Use TODO when something needs to be completed but working. Code that is not implemented or not tested should raise an exception. For every instance add necessary comment to further explain the situation.

Note: See TracWiki for help on using the wiki.