pegasus/doc/CodingGuidelines.htm - diff

Return to CodingGuidelines.htm CVS log

Up to [Pegasus] / pegasus / doc

Diff for /pegasus/doc/CodingGuidelines.htm between version 1.4 and 1.5

version 1.4, 2013/05/03 05:34:37

version 1.5, 2013/12/02 12:13:46

Line 1

<html>

<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">

<head>

<title>PEP 221 - Pegasus Coding Conventions</title>

</head>

<body>

<hr>

Line 16

Line 14

Title: Pegasus Coding Conventions

Status: Approved

Version History:

<table border="1" cellspacing="1" bordercolordark="#666666"

bordercolorlight="#CCCCCC" width=100%

style='font-size=10.0pt;font-family:Arial'>

<th bgcolor="#cae6ca">Version</th>

<tr>

<th bgcolor="#CAE6CA">Version</th>

<th bgcolor="#cae6ca">Author</th>

<th bgcolor="#cae6ca">Change Description</th>

<th bgcolor="#CAE6CA">Author</th>

<th bgcolor="#CAE6CA">Change Description</th>

</tr>

<tr>

Line 110

Line 106

<td align="center">22 January 2013</td>

<td align="center">Marek Szermutzky</td>

<td>macro PEGASUS_FCT_EXECUTE_AND_ASSERT added</td>

</tr><tr>

<td style="vertical-align: top; text-align: center;">3.6

</td>

<td style="vertical-align: top; text-align: center;">19 November 2013

</td>

<td style="vertical-align: top;">Karl Schopmeyer

</td>

<td style="vertical-align: top;">Add rule about defaults in switch constructs (see Formatting, 22)

</td>

</tr>

</table>

</tbody></table>

<hr>

Abstract: This document formalizes the Pegasus coding conventions.

Line 119

Line 125

<hr>

<h2>Definition of the Problem</h2>

<The use of inconsistent coding styles is an impediment to

effective development and maintenance of Pegasus code. Formalization of

Pegasus coding conventions may help to improve code consistency.

<h2>Proposed Solution</h2>

The following items comprise the Pegasus coding conventions.

<h3>Formatting</h3>

<ol>

<li>Indent by increments of four spaces. Indent comments

equally with the associated code.</li>

<li>Do not use tab characters.</li>

Line 139

Line 143

<li>Put opening brace on a line by itself, aligned with control keyword

or method signature.

Do this:

<pre><code> for (...)

{

}</code></pre>

Not this:

<pre><code> for (...) {

}</code></pre>

Or this:

<pre><code> for (...)

{

}</code></pre>

</li>

<li>Use braces around the body of a control block (e.g., <code>if</code>,

<code>for</code>, or <code>while</code> statement), even if the body

contains just a single statement.

Do this:

<pre><code> for (...)

{

i++;

}</code></pre>

Not this:

<pre><code> for (...)

i++;</code></pre>

</li>

<li>Use a new line for <code>else</code> statements rather

than placing then on the same line as a preceding '<code>}</code>'.

Do this:

<pre><code> if (...)

{

Not this:

i++;

}

else

{

j++;

}</code></pre>

Not this:

<pre><code> if (...)

{

i++;

} else

{

j++;

}</code></pre>

</li>

<li>Use normal indenting for parameters in a complex method signature:

<pre><code> void MyClass::myMethod(

<pre><code> void MyClass::myMethod( const char* someReallyLongName, const char* someOtherReallyLongName);</code></pre>

const char* someReallyLongName,

const char* someOtherReallyLongName);</code></pre>

</li>

<li>Use normal indenting for all arguments in a complex function call.

Do this:

<pre><code> callingMyFunction(

<pre><code> callingMyFunction( arg1, arg2, arg3);</code></pre>

arg1,

Not this:

arg2,

<pre><code> callingMyFunction(arg1, arg2, arg3);</code></pre>

arg3);</code></pre>

Not this:

<pre><code> callingMyFunction(arg1,

arg2,

arg3);</code></pre>

Each argument should be placed on a separate line.

An exception is made for <code>PEG_TRACE</code> and

<code>PEG_TRACE_CSTRING</code> statements, where the first two

arguments should be placed on the opening line:

<pre><code> PEG_TRACE((TRC_HTTP, Tracer::LEVEL3,

<pre><code> PEG_TRACE((TRC_HTTP, Tracer::LEVEL3, "Connection IP address = %s", (const char*)_ipAddress.getCString()));</code></pre>

"Connection IP address = %s",

(const char*)_ipAddress.getCString()));</code></pre>

</li>

<li>For an <code>if</code>, <code>while</code>, or <code>for</code>

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:

<pre><code> if ((variable1 == variable2) ||

<pre><code> if ((variable1 == variable2) || ((anotherVariableToConsider == variable1 + variable2) && (dayOfWeek == "Tuesday")))</code></pre>

((anotherVariableToConsider ==

Not this:

variable1 + variable2) &&

<pre><code> if ((variable1 == variable2) || ((anotherVariableToConsider == variable1 + variable2) && (dayOfWeek == "Tuesday")))</code></pre>

(dayOfWeek == "Tuesday")))</code></pre>

Not this:

<pre><code> if ((variable1 == variable2) ||

((anotherVariableToConsider ==

variable1 + variable2) &&

(dayOfWeek == "Tuesday")))</code></pre>

</li>

<li>For a statement that does not fit on a single line,

all continuation lines should be indented once from the first line.

For example:

<pre><code> sum = a + b + c +

d + e + f +

g + h;</code></pre>

</li>

<li>Do not separate a return type onto its own line. Avoid this:

<pre><code> int

f()

{

}</code></pre>

</li>

<li>Avoid use of "<code>(void)</code>" for an empty parameter list.

Use "<code>()</code>" instead.

</li>

<li>Avoid adding a space between function/method name the opening parenthesis.

Do this:

<pre><code> void f(int i);

f(10);</code></pre>

Not this:

<pre><code> void f (int i);

f (10);</code></pre>

</li>

<li>Include a space between a keyword and an opening parenthesis.

Do this:

<pre><code> if (i > 0)

for (i = 0; i < 10; i++)</code></pre>

Not this:

<pre><code> if(i > 0)

for(i = 0; i < 10; i++)</code></pre>

</li>

<li>Do not add spaces around a condition. Do this:

<pre><code> if (cond)

<pre><code> if (cond) while (cond)</code></pre>

while (cond)</code></pre>

Not this:

<pre><code> if ( cond )

<pre><code> if ( cond ) while ( cond )</code></pre>

while ( cond )</code></pre>

</li>

<li>Do not add a space between a template name and its type. Do this:

<pre><code> Array<Uint8> myArray;</code></pre>

Line 264

Line 220

<pre><code> Array <Uint8> myArray;</code></pre>

</li>

<li>Avoid aligning variable names in a declaration list. Do this:

<pre><code> int x;

<pre><code> int x; float y;</code></pre>

float y;</code></pre>

Not this:

<pre><code> int x;

<pre><code> int x; float y;</code></pre>

float y;</code></pre>

</li>

<li>Avoid indenting "<code>public:</code>",

"<code>protected:</code>", and

"<code>private:</code>". Do this:

<pre><code> class X

<pre><code> class X { public: int f(); ... private: int _g(); ... };</code></pre>

{

public:

int f();

...

private:

int _g();

...

};</code></pre>

</li>

<li>The '<code>#</code>' indicating a preprocessing directive is placed

at the beginning of a line. Nested preprocessing directives are

indented with one space after the '<code>#</code>' for each level of

nesting. The <code>#ifndef</code> used for header file include protection

is not considered a level of nesting. For example:

<pre><code>#ifdef PEGASUS_HAS_SIGNALS

<pre><code>#ifdef PEGASUS_HAS_SIGNALS # ifdef PEGASUS_OS_TYPE_WINDOWS # include <windows.h> # else # include <unistd.h> # endif #endif</code></pre>

# ifdef PEGASUS_OS_TYPE_WINDOWS

# include <windows.h>

# else

# include <unistd.h>

# endif

#endif</code></pre>

</li>

<li>In a <code>switch</code> statement, indent the <code>case</code>

statement from the <code>switch</code> and indent the case logic from

the <code>case</code> statement. For example:

<pre><code> switch(n)

<pre><code> switch(n) { case 0: printf("Rock"); break; case 1: printf("Paper"); break; case 2: printf("Scissors"); break; default: printf("Error"); break; }</code></pre></li>

{

<li>A <code>switch </code>construct for an enumerated type the

case 0:

construct MUST contain either all of the possible cases  or the

printf("Rock");

default statement but not both. We encourage including all of the

break;

enumerated types since this provides a compile time check if the

case 1:

enumerated type changes.  Do not include the default statement if

printf("Paper");

all of the possible enumerated types are included in the <code>case</code>

break;

statements. At least in gcc this is enforced in the standard build with

case 2:

the -Werror=switch flag which generates an error statement when the <code>switch</code> has an index of enumerated type

printf("Scissors");

and lacks a <code>case</code> for one or more of the named codes of that

break;

enumeration.</li>

default:

printf("Error");

break;

}</code></pre>

</li>

<li>Do not put parentheses around the return value in a

<code>return</code> statement.

</li>

</ol>

<h3>Naming</h3>

<ol>

<li>For class/struct names, use mixed case with initial upper case and

no underscores: <code>ThisIsAClassName</code>.

</li>

Line 348

Line 286

<li>Environment variables must begin with <code>PEGASUS_</code> and have

this form: <code>PEGASUS_ENVIRONMENT_VARIABLE</code>.

This applies to environment variables that control the build and test

configuration as well as those (if any) used by Pegasus at run time.

configuration as well as those (if any) used by Pegasus at run time by forcing an error if the 

</li>

<li>Test executable names should begin with 'Test' (to distinguish them

from other executables) and should use mixed case with initial upper

Line 357

Line 295

</li>

</ol>

<h3>Style</h3>

<ol>

<li>In a header file, use angle brackets (with a fully qualified path)

rather than quotes when including a file. Do this:

<pre><code> #include <Pegasus/Common/Array.h></code></pre>

Not this:

<pre><code> #include "Array.h"</code></pre>

</li>

<li>Use "<code>0</code>" rather than "<code>NULL</code>"

as a null pointer value.

</li>

<li>Avoid <code>throw()</code> declarations. These may be used

Line 385

Line 324

String construction:

<pre><code> String s = foo();</code></pre>

while this logic causes two String constructions and an assignment:

<pre><code> String s;

<pre><code> String s; ... s = foo();</code></pre>

...

s = foo();</code></pre>

</li>

<li>Present resources as part of classes, do not use them directly.</li>

<li>Use <code>new</code> and <code>delete</code> to manage dynamic

Line 397

Line 334

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:

<pre><code> catch (Exception&)

<pre><code> catch (Exception&) { ... throw; }</code></pre>

{

Not this:

...

<pre><code> catch (Exception& e) { ... throw e; }</code></pre></li>

throw;

}</code></pre>

Not this:

<pre><code> catch (Exception& e)

{

...

throw e;

}</code></pre></li>

<li>Do not check whether a pointer is non-null before

deleting it. The check is unnecessary, since it is also performed by

the <code>delete</code> operator. Do this:

<pre><code> delete ptr;</code></pre>

Not this:

<pre><code> if (ptr)

<pre><code> if (ptr) { delete ptr; }</code></pre></li>

{

delete ptr;

}</code></pre></li>

<li>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

Line 432

Line 358

</li>

<li>Pass object parameters by reference, when feasible,

to avoid unnecessary copy constructions. For example, this:

<pre><code> void func(const String& s);</code></pre>

is preferred to this:

<pre><code> void func(String s);</code></pre></li>

<li>Avoid using the C++ standard template library (STL) and the standard

Line 464

Line 390

comments.

</li>

<li>Use this comment style for DOC++ comments:

<pre><code> class X

<pre><code> class X { public: /** Creates widgets. @param numWidgets The number of widgets to create. @return true if successful, false otherwise. */ Boolean createWidgets(Uint32 numWidgets); };</code></pre>

{

public:

/**

Creates widgets.

@param numWidgets The number of widgets to create.

@return true if successful, false otherwise.

Boolean createWidgets(Uint32 numWidgets);

};</code></pre>

</li>

<li>A description should end with a period, even if it is brief:

<pre><code> /**

<pre><code> /** Does something useful. */</code></pre>

Does something useful.

*/</code></pre>

</li>

<li>Add a blank line between documented methods:

<pre><code> /**

<pre><code> /** Does something quite useful. */ void f(); /** The previous method has a newline after it to improve its visibility. */ void g();</code></pre>

Does something quite useful.

void f();

/**

The previous method has a newline after it

to improve its visibility.

void g();</code></pre>

</li>

</ol>

Line 525

Line 430

define a <code>PEGASUS_<LIBRARY>_LINKAGE</code> symbol in a

Linkage.h file. Each symbol that is exported from the library

must be declared with this linkage macro. For example:

<pre><code> class PEGASUS_COMMON_LINKAGE String;

<pre><code> class PEGASUS_COMMON_LINKAGE String; PEGASUS_COMMON_LINKAGE void globalFunction();</code></pre>

PEGASUS_COMMON_LINKAGE void globalFunction();</code></pre>

</li>

<li>A <code>main()</code> function must return an int (required by

Windows NT).

Line 539

Line 442

<pre><code> void f() throw TooBad;</code></pre>

</li>

<li>Do not include a definition of a static member with its declaration:

<pre><code> class X

<pre><code> class X { public: static const Uint32 COLOR = 225; };</code></pre>

{

public:

static const Uint32 COLOR = 225;

};</code></pre>

Use this instead:

<pre><code> class X

<pre><code> class X { public: static const Uint32 COLOR; };</code></pre>

{

public:

static const Uint32 COLOR;

};</code></pre>

And add the definition in the source file:

<pre><code> const Uint32 X::COLOR = 225;</code></pre>

</li>

Line 559

Line 452

printf and scanf conversions of 64-bit integers rather than

<code>ll</code> or <code>L</code>.

Do this:

<pre><code> Sint64 i64 = 10;

<pre><code> Sint64 i64 = 10; printf("i64 value = %" PEGASUS_64BIT_CONVERSION_WIDTH "d.\n", i64);</code></pre>

printf("i64 value = %" PEGASUS_64BIT_CONVERSION_WIDTH "d.\n", i64);</code></pre>

Instead of this:

<pre><code> Sint64 i64 = 10;

<pre><code> Sint64 i64 = 10; printf("i64 value = %lld.\n", i64);</code></pre>

printf("i64 value = %lld.\n", i64);</code></pre>

</li>

<li>Do not include class scoping on methods or members in a class definition.

Do this:

<pre><code> class X

<pre><code> class X { public: int myMethod(); };</code></pre>

{

Not this:

public:

<pre><code> class X { public: int X::myMethod(); };</code></pre>

int myMethod();

};</code></pre>

Not this:

<pre><code> class X

{

public:

int X::myMethod();

};</code></pre>

</li>

<li>Do not use <code>Pegasus::</code> to scope symbols. Not all platforms

support C++ namespaces, and this will cause a compilation failure. If

Line 604

Line 485

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:

<pre><code>int main(int, void** argv)

{

}</code></pre>

Not this:

<pre><code>int main(int argc, void** argv)

{

}</code></pre>

</li>

<li>Avoid unused variables when the return value of a function is

assigned to a variable only to be used to do an assert check

Line 621

Line 498

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:

<pre><code>PEGASUS_FCT_EXECUTE_AND_ASSERT(true, f());

<pre><code>PEGASUS_FCT_EXECUTE_AND_ASSERT(true, f()); </code></pre>

</code></pre>

Not this:

<pre><code>bool returnCode = f(); PEGASUS_ASSERT(true == returnCode);

<pre><code>bool returnCode = f(); PEGASUS_ASSERT(true == returnCode); </code></pre>

</code></pre>

</li>

</ol>

Line 633

Line 508

<h2>Discussion</h2>

Suggestions for additional conventions are listed here for discussion:

<ul>

<ul>

<li>Handling of error conditions (cerr vs. logging) and exceptions</li>

<li>Testing (perhaps reference another document)</li>

<li>Is it appropriate to make a statement about defining

localizable messages in the appropriate message bundle?

</li>

</ul>

<hr>

Line 650

Line 525

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

Line 659

Line 534

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

Line 670

Line 545

Template last modified: March 9th 2004 by Martin Kirk

Template version: 1.8

</body>

</body></html>

</html>

Legend:

Removed from v.1.4
changed lines
	Added in v.1.5

No CVS admin address has been configured