Page History

...

Statements

Types

5-1. Types that are local to a single .cc file only SHOULD be declared inside that file.

For example, if a type is declared locally within a class body then the declaration goes within the .cc file, not the .h file.

Enforces information hiding.

5-2. The parts of a class MUST be sorted 'public' , 'protected' , and 'private'.

All sections must be identified explicitly. Not applicable sections may be left out. Member declarations should be done with data members first, then member functions, in each section:

class MyClass{
public:
    int anInt;
    int doSomething();
protected:
    float aFloat;
    float doSomethingElse();
private:
    char _aChar;
    char doSomethingPrivately();
  ...
}

The ordering is "most public first" so people who only wish to use the class can stop reading when they reach the protected/private sections. There must be at most one 'public' , one 'protected' and one 'private' section in the 'class' declaration.

5-2b. A class or struct definition MUST explicitly declare the privacy qualifier of its base classes.

A class or struct definition must explicitly declare the privacy qualifier of its base classes.

struct derived : public base {};
class d : private b {};

Although C++ provides the above definitions as defaults, some compilers generate warnings if explicit privacy qualifiers are not specified. This Rule will reduce unnecessary compiler warnings.

5-3. Type conversions SHOULD be avoided as far as possible.

When required, type conversions MUST always be done explicitly using C++ style casts.

Never rely on implicit type conversion.

floatValue = static_cast<float> (intValue);    // YES!
floatValue = intValue;                         // NO!
floatValue = (float)intValue;                  // NO C-style casts!

By this, the programmer indicates that he is aware of the different types involved and that the mix is intentional. If you find you are casting a lot, stop and think! Maybe there is a better way to do things.

Variables

...

int i=0;
float aFloat = 0.0;
int *i = 0          // 0 preferred pointer initialization, not NULL

This ensures that variables are valid at any time. Sometimes it is impossible to initialize a variable to a valid value where it is declared:

int x, y, z;
getCenter(&x, &y, &z);

In these cases it may be left uninitialized rather than initialized to some phony value. Fixed phony values can be of use in debugging since they are consistent across runs, machines, builds and platforms. See also Rule 5-13.

5-5. Multiple assignment SHOULD be used only with a constant type.

// OK:
float a, b, c;
a = b = c = 8675.309;

// NOT OK:
std::string a;
int b;
double c;
a = b = c = 0;

Multiple assignment seems harmless when considering the first example. However, while the second example is legal C++ (although it will generate compiler warnings), mixing types in assignment statements can lead to unintended results later.

5-6. Variables MUST never have dual meaning.

Enhance readability by ensuring all concepts are represented uniquely. Reduce chance of error by side effects.

...

In C++ there is no reason that global variables need to be used at all. The same is true for global functions or file scope (static) variables. See also Rule 3-9.

5-8. Non-constant and instance variables MUST be declared private.

Public members are allowed only if declared both 'const' and 'static'. The concept of C++ information hiding and encapsulation is violated by public variables. If access to data members is required, then this must be provided through public or protected member functions. The argument for public variables is generally one of efficiency. However, by declaring the accessor and mutator functions in-line, efficiency can be regained.

One exception to this rule is when the class is essentially a data structure, with no behavior (equivalent to a C 'struct' ). In this case it is acceptable to make the class's instance variables public. Note that 'struct s' are kept in C++ for compatibility with C only, and avoiding them increases the readability of the code by reducing the number of constructs used. Use a class instead.

5-9. Related variables of the same type MAY be declared in a common statement.

Unrelated variables should not be declared in the same statement.

float  x, y, z;
float  revenueJanuary, revenueFebruary, revenueMarch;

The common requirement of having declarations on separate lines is not useful in the situations like the ones above. It enhances readability to group variables like this.

5-10. The 'const' keyword SHOULD be listed after the type name.

void f1(Widget const *v)     // NOT: void f1(const Widget *v)

This is for a mutable pointer to an immutable Widget. Stroustrup points out one advantage to this order: you can read it from right to left i.e. "v is a pointer to a const Widget". Of course this is different than:

Widget * const p

Which is an immutable pointer to a mutable Widget. Again, the right-to-left reading is pretty clear, so this and the above reinforce each other.

5-11. Implicit test for 0 SHOULD NOT be used other than for boolean variables.

if (nLines != 0)    // NOT:   if (nLines)

By using explicit test the statement gives an immediate clue of the type being tested. It is common also to suggest that pointers shouldn't test implicit for 0 either, i.e. 'if (line == 0)' instead of 'if (line)' . The latter is regarded as such a common practice in C/C++ however that it can be used.

5-12. Floats and doubles SHOULD NOT be tested for equality unless the comparison is to zero.

// NO
if (value == 1.0)    // Subject to roundoff error

// PREFERRED
if (fabs(value - 1.0) < std::numeric_limits<float>::epsilon()) {
    ...
}

// OK in specific situations
if (b == 0.0 && sigma2 == 0.0) {
    _sigma2 = 1.0;    //avoid 0/0 at center of PSF
}

Round-off makes it difficult for two floating point numbers to be truly equal. Always use greater than or less than. A utility method like 'boolean closeEnough(value1,value2)' may be useful for particular cases (e.g. to compare two Images)

...

Variables should be initialized when declared (and not declared before they can be initialized).

By keeping the operations on a variable within a small scope, it is easier to control the effects and side effects of the variable. See also Rule 5-4.

Loops

5-14. Loop variables SHOULD be declared in loop scope. Prefer pre- increment & decrement.

Use of pre-increment and pre-decrement is preferred but not required. Loop variables should be declared in loop scope when possible (it isn't possible if they have different types, such as ptr and x in the example). It is permissible to advance more than one variable in the loop control part of the for if this makes logical sense (e.g. if you're advancing iterators through two arrays simultaneously, or needing to know the coordinates of a pixel iterator).

// YES:
int sum = 0;
double x = 0.0;
for (iter ptr = vec.begin(), end = vec.end();  ptr != end; ++ptr, ++x) {
    sum += x*(*ptr);
}

// NO:
int sum = 0;
for (int i = 0; i < 100; i++) {
    sum += value[i];
}

If you write 'iter++', the method is required to make a copy of 'iter' before incrementing it, as the return value is the old value. If 'iter' is a pointer this is cheap and probably inlined (and thus optimized away) but for complex objects it can be a significant cost. The convention for STL code is to always pre-increment, and we should follow it. See e.g. Meyers, More Effective C++, item 6.

This is only a recommendation; there are times when you do need the old value, and in that case postfix ++ is exactly what you want.

Increase maintainability and readability. Make it crystal clear what controls the loop and what the loop contains.

5-15. Loop variables SHOULD be initialized immediately before the loop.

// YES:
bool isDone = false;
while (!isDone) {
    doSomething();
}

// NO: Don't separate loop variable initialization from use
bool isDone = false;
[....lots of code here...]
while (!isDone) {
    doSomething();
}

5-16. 'do-while' loops SHOULD be avoided.

'do-while' loops are less readable than ordinary 'while' loops and 'for' loops since the conditional is at the bottom of the loop. The reader must scan the entire loop in order to understand the scope of the loop.

In addition, 'do-while' loops are not needed. Any 'do-while' loop can easily be rewritten into a 'while' loop or a 'for' loop. Reducing the number of constructs used enhance readability.

5-17. (Deleted)

5-18. The form 'while(true)' SHOULD be used for infinite loops.

while (true) {
    doSomething();
}

for (;;) { // NO!
    doSomething();
}

while (1) { // NO!
    doSomething();
}

Testing against 1 is neither necessary nor meaningful. The form 'for (;;)' is not as apparent that this actually is an infinite loop.

Conditionals

5-19. Complex conditional expressions SHOULD be avoided. Introduce temporary boolean variables instead.

if ((elementNo < 0) || (elementNo > maxElement)||


    elementNo == lastElement) {
    ...
}

should be replaced by:

bool const isFinished    = (elementNo < 0) || (elementNo > maxElement);
bool const isRepeatedEntry = elementNo == lastElement;
if (isFinished || isRepeatedEntry) {
    ...
}

By assigning boolean variables to expressions, the program gets automatic documentation. The construction will be easier to read and to debug.

5-20. The nominal case SHOULD be put in the 'if' -part and the exception in the 'else' -part of an 'if' statement.

int nChar;
nChar = readFile(fileName);
if (nChar > 0) {
    ...
} else {
    ...
}

Makes sure that the exceptions don't obscure the normal path of execution. This is important for both the readability and performance.

5-21. The conditional MAY be put on a separate line.

//YES:
if (isDone) {
  doCleanup()
}

// Also OK:
if ( isDone) doCleanup();

This is useful when using a symbolic debugger: when written on a single line, it is not apparent whether the test is true or not.

5-22. Executable statements in conditionals MUST be avoided.

// NO!
if ((fileHandle = open (fileName, "w"))) {
    ...
}

// YES:
fileHandle = open (fileName, "w");
if (fileHandle) {
    ...
}

Conditionals with executable statements are just very difficult to read. This is especially true for programmers new to C/C++.

Methods and Functions

5-23. Functions MUST always have the return value explicitly listed.

// YES:
int getValue() {
    ...
}

// NO:
getvalue() {
}

If not explicitly listed, C++ implies int return value for functions. A programmer must never rely on this feature, since this might be confusing for programmers not aware of this artifact.

5-23b. Unused method and function arguments MUST be unnamed.

void MyDerivedClass::foo( double /* scalefactor */ ) { // OK
    };
 
    void MyDerivedClass::foo( double ) { // OK
    };

This is common in template specializations and derived methods, where a variable is needed for some cases but not all. In order to remind the developer of the significance of the missing parameter, an in-line C comment may be used.

Although C++ allows omission of an unused argument's name, some compilers generate warnings if a named argument is not accessed. This Rule will reduce unnecessary compiler warnings.

5-24. Arguments that are of non-primitive types and will not be modified SHOULD be passed by 'const' reference.

void setWidget(Widget const &widget)

Passing by 'const' reference when possible is much more efficient than passing large objects but also allows use of non-pointer syntax in the method.

5-24b. Smart pointers (such as shared_ptr) should only be used as arguments if a reference or const reference cannot be used.

Examples of when a smart pointer argument is appropriate include when the pointer itself may be reset, when a null pointer is considered a valid input, and when the pointer (not the pointee) will be copied and held after after the function returns (as in a constructor or member function setter). In all other cases, reference or const reference arguments should be used.

Motivation: it is difficult and sometimes expensive to create a smart pointer from a reference or plain value, so a smart pointer should not be required to call a function unless necessary.

5-25. Class methods that do not update internal data nor return references/pointers to internal data MUST use the 'const' label at the end of the signature.

double getFactor() const;

This is required if one wants to manipulate constant versions of the object.

5-26. All methods that return references/pointers to internal data MUST provide both a constant and non-constant version when appropriate.

Use the 'const' version where possible.

Antenna& getAntenna(unsigned int i);
Antenna const & getAntenna(unsigned int i) const;

The first example returns internal data. If the class containing the function is constant, you can only call functions that have the trailing 'const' label. To call a function without the label is a compile-time error. For example:

class Telescope {
    Antenna& getAntenna(unsigned int i);
};

const Telescope tel = obs.getTelescope();
Antenna const & ant = tel.getAntenna(1); // ERROR!

Constructors and Destructors

5-27. Constructors taking one argument MUST be declared as 'explicit' .

A default constructor must be provided. Avoid implicit copy constructors.

class Year {
private:
    int y;
public:
    explicit Year(int i) : y(i) { }
};

    Year y1 = 1947; // illegal
    Year y2 = Year(1947); // OK
    Year y3(1947);  // Better

// Example of unintended result and no error reported
class String {
    int size;
    char *p;
public:
    String(int sz);    // constructor and implicit conversions
}
void f(){
    String s(10);
    s = 100;         // programmer's typo not detected; 100 is
                     // converted to a String and then assigned to s!
}

This avoids implicit type conversions (see Rule 5-3). The declaration of y1 would be legal had 'explicit' not been used. This type of implicit conversion can result in incorrect and unintentional side effects.

5-28. Destructors MUST NOT throw exceptions.

This is a violation of the C++ standard library requirements (see Stroustrup Appendix E.2).

5-29. Destructors SHOULD be declared virtual in polymorphic base classes.

Paraphrasing Item 7 in Scott Meyer's Effective C++, 55 Specific Ways to Improve Your Programs and Designs:

"The rule for giving base classes virtual destructors applies only to base classes designed to allow the manipulation of derived class types through base class interfaces; such classes are known as 'polymorphic' base classes.

• Polymorphic base classes should declare virtual destructors. If a class has any virtual functions, it should have a virtual destructor.
• Classes not designed to be base classes or not designed to be used polymorphically should not declare virtual destructors."

In the example below, without a virtual destructor, the proper destructor will not be called.

class Base
{
public:
    Base() { }
    ~Base() {}      // Should be:   virtual ~Base() { }
};

class Derived: public Base
{
public:
     Derived() { }
     ~Derived() { }
};

void main()
{
    Base *b = new Derived();
    delete b;      // Will not call Derived::~Derived() unless 'virtual ~Base()' was defined !
}

Miscellaneous

5-30. The use of magic numbers in the code SHOULD be avoided.

If the number does not have an obvious meaning by itself, the readability is enhanced by introducing a named constant instead (see Rule 3-3). A different approach is to introduce a method from which the constant can be accessed.

5-31. Floating point constants SHOULD always be written with decimal point and with at least one decimal.

double total = 0.0;   // NOT: double total = 0;
double speed = 3.0e8; // NOT: double speed = 3e8;

double a;
double b;
...
double const SOME_GOOD_NAME = 10.0d;
double sum = (a + b) * SOME_GOOD_NAME;

This emphasizes the different nature of integer and floating point numbers even if their values might happen to be the same in a specific case. Although integers cannot be written using exponential notable (second example), for consistency we recommend using the decimal and trailing zero. Also, as in the last example above, it emphasizes the type of the assigned variable (sum) at a point in the code where this might not be evident.

5-32. Floating point constants SHOULD always be written with a digit before the decimal point.

double gainOffset(0.5);   // NOT: double gainOffset(.5);

The number and expression system in C++ is borrowed from mathematics and one should adhere to mathematical conventions for syntax wherever possible. Also, 0.5 is a lot more readable than .5; There is no way it can be mixed with the integer 5.

5-33. goto SHOULD NOT be used.

'Goto' statements violate the idea of structured code. Only in some very few cases (for instance breaking out of deeply nested structures) should goto be considered, and only if the alternative structured counterpart is proven to be less readable.

5-34. "0" SHOULD be used instead of "NULL".

'NULL' is part of the standard C library, but is made obsolete in C++. However, it is still frequently used, so 'NULL' is permitted as long as it is not typedef-ed or '#define-'d to something other than 0.

5-35. Signed int SHOULD be the preferred type for indices, even those in which a negative value is illegal.

double d = new d[10];
for (int i = 0; i < 10; i++) { d[i] = static_cast<double>(i); }

'unsigned int' helps avoid index out of range exceptions at compile-time, but it throws you a curve when comparing ints and unsigned ints; requiring you to explicitly cast unsigned to signed.

5-36. Exceptions MUST NOT be declared in method signatures, and all exceptions MUST be documented with the '@throw' tag.

Use of 'throw' in a signature does not encourage robust handling of exceptions. The ramifications of declaring exceptions are spelled out in Stroustrup (3rd ed.) in section 14.6. A few rules of thumb:

1. A function declaration without a "throw" can throw any exception 
2. A declaration containing a "throw" can only throw the listed exceptions. 
3. Any exception not matching one of the declared exceptions (or a subclass of it) will be automatically rerouted to std::unexpected(). The default implementation of this function is to call std::terminate() (which calls std::abort()).
4. Throw() may be used to indicate that no exceptions are expected to be thrown.

Exceptions thrown by a class should be apparent to a user of that class. Hence the '@throw' requirement.

5-36b. Unused exception variables MUST be unnamed.

try {
    } catch (ExceptionClass &) {      // OK
    };

Although C++ allows omission of the variable name, some compilers generate warnings if a named variable is not accessed. This Rule will reduce unnecessary compiler warnings.

5-37. '#define' statement use SHOULD be minimized.

// Preferred
int const A_POWER_OF_TWO = 16;

// NO
#define A_POWER_OF_TWO 16

They have subtle side effects in debuggers and other tools. For example, symbolic names for constants aren't visible to the debugger and require const variables.

5-38. No code SHOULD be commented out; use a preprocessor directive to include or inhibit code use.

Specifically for debug print statements, use the 'lsst::pex::log::Trace' class.

#define DEBUG_IO 1
#if defined(DEBUG_IO)
    [...statements...]
#endif

5-39. 'std::String' class SHOULD be used rather than 'char *'

We are developing in C++ not C, let's use the quite good standard string class.

5-40. 'std::vector<Foo>' SHOULD be used preferentially to array declaration (e.g. Foo[]).

This is less prone to memory leaks (i.e. putting delete instead of delete[]) and you don't need special pointers to work with it. Again, let's use the good STL classes.

5-41. 'using namespace' SHOULD be minimized when defining symbols

'Using namespace' should only be used for system library 'std'.

#include iostream.
using namespace std;

It can be difficult to determine from where a particular symbol came.

5-42. A definition or abbreviated namespace SHOULD be used when defining symbols

It is strongly recommended to use a definition or abbreviated namespace name, as in:

# Specify namespace explicitly in the definition
lsst::foo::bar::myFunction(...) {...};

# Use an abbreviation for the namespace
namespace fooBar lsst::foo::bar;
fooBar::myFunction(...) {...};

As a matter of policy, the module's developer should define the abbreviation to be used throughout the LSST codeset in the module's source file(s).

Uniformity of namespace abbreviation name across the full codeset makes code easier to quickly understand.

See Rule 3-6 for an almost equivalent Rule.

5-43. Implementation-specific globals SHOULD go in namespace *::detail

Sometimes implementation-specific details need to be globally visible (i.e. can't be in the private part of a class, or be declared static or in an anon namespace in a single file).

For example, the fits i/o code in lsst::afw::image uses boost::gil internals but needs to be in a header file included by both Image.cc and Mask.cc; there are also Image traits classes. In keeping with the boost convention, such global information should be consigned to a *::detail namespace (in this case, lsst::afw::image::detail).

We should, of course, strive to minimize the amount of such information.