Most crucial elements in a light-weight C++ coding standard

Question

I've been involved in developing coding standards which were quite elaborate. My own experience is that it was hard to enforce if you don't have proper processes to maintain it and strategies to uphold it.

Now I'm working in, and leading, an environment even less probable to have processes and follow-up strategies in quite a while. Still I want to uphold some minimum level of respectable code. So I thought I would get good suggestions here, and we might together produce a reasonable light-weight subset of the most important coding standard practices for others to use as reference.

So, to emphasize the essence here:

What elements of a C++ coding standard are the most crucial to uphold?

• Answering/voting rules

  • 1 candidate per answer, preferably with a brief motivation.

  • Vote down candidates which focuses on style and subjective formatting guidelines. This is not to indicate them as unimportant, only that they are less relevant in this context.

  • Vote down candidates focusing on how to comment/document code. This is a larger subject which might even deserve its own post.

  • Vote up candidates that clearly facilitates safer code, which minimizes the risk of enigmatic bugs, which increases maintainability, etc.

  • Don't cast your vote in any direction on candidates you are uncertain about. Even if they sound reasonable and smart, or on the contrary "something surely nobody would use", your vote should be based on clear understanding and experience.

Answer 1

If the toolchain in use (or projected use) has an inefficient implementation of exceptions, it might be wise to avoid their use. I've worked under such conditions.

Update: here is someone else's rationale for "Embedded C++", which seems to exclude exceptions. It makes the following points:

• It is difficult to estimate the time between when an exception has occurred and control has passed to a corresponding exception handler.
• It is difficult to estimate memory consumption for exception handling.

There is more elaborate text on that page, I didn't want to copy it all. Plus, it's 10 years old so it might be of no use any longer, which is why I included the part about the toolchain. Perhaps that should also read "if memory is not considered a major problem", and/or "if predictable real-time response is not required", and so on.

Answer 2

Method and variable names in a common naming scheme for consistency; I don't tend to be bother much by anything else while reading source.

Answer 3

No tabs (allows better use of external/other tools) and a fixed spaces inserted for tabs.

Answer 4

Curly braces required if you have more than one step of indentation:

if (bla) {
  for (int i = 0; i < n; ++i)
    foo();
}

This helps to keep indentation in line with how the compiler sees the code.

Answer 5

Keep functions to a reasonable size. Personally, I like to keep functions under 25 lines. Readability is enhanced when you can take a function in as a unit rather than having to scan up and down trying to figure out how it works. If you have to scroll to read it, it makes matters even worse.

Answer 6

Only trivial use of the ? : operator, i.e.

float x = (y > 3) ? 1.0f : -1.0f;

is ok, but this is not:

float x = foo(2 * ((y > 3) ? a : b) - 1);

Answer 7

Sort functions in class declarations and definitions by name. This makes it easier to locate them in the .cpp file. Also, it frees your mind because you don't have to think about where to put your new function.

Answer 8

Make sure that your compiler's warning level is set high enough (/Wall preferably) so that it will catch silly mistakes like:

if (p = 0)

when you really meant

if (p == 0)

so that you don't need to resort to even sillier tricks like:

if (0 == p)

which degrade the readability of your code.

Answer 9

assert all assumptions, including temporary assumptions, like unimplemented behavior. assert function entry and exit conditions if nontrivial. assert all nontrivial intermediate states. your program should never crash without an assert failing first. you can customize your assert mechanism to ignore future occurances.

Use error-handling code for conditions you expect to occur; use assertions for conditions that should never occur. Error handling typically checks for bad input data; assertions check for bugs in the code.

If error-handling code is used to address an anomalous condition, the error handling will enable the program to respond to the error gracefully. If an assertion is fired for an anomalous condition, the corrective action is not merely to handle an error gracefully—the corrective action is to change the program's source code, recompile, and release a new version of the software. A good way to think of assertions is as executable documentation—you can't rely on them to make the code work, but they can document assumptions more actively than program-language comments can [1].

1. McConnell, Steve. Code Complete, Second Edition. Microsoft Press © 2004. Chapter 8 - Defensive Programming

Answer 10

Curly braces for any control statement. (Thanks to own experience and reinforced by reading Code Complete v2):

// bad example - what the writer wrote
if( i < 0 ) 
    printf( "%d\n", i );
    ++i; // this error is _very_ easy to overlook!  

// good example - what the writer meant
if( i < 0 ) {
    printf( "%d\n", i );
    ++i;
}

Answer 11

Prefer RAII.

STL's auto (and shared in boost & C++0x) pointers may help.

Answer 12

Use references instead of pointers where possible. This prevents constant defensive NULL checks.

Answer 13

Use a lint tool - i.e. PC-Lint. This will catch many of the 'structural' coding guideline issues. Meaning things that read to actual bugs rather than style/readability issues. (Not that readability is not important, but it is less so than actual errors).

Example, rather than requiring this style:

if (5 == variable)

As a way of preventing the 'unintended assignment' bug, let lint find it.

Answer 14

Whatever guidelines, make it very easy to recognize applicability: the less choice you have, the less time you loose choosing. And the easier it becomes to brainparse the code.

Examples of 'hard to recognize':

• No braces if only one line in the conditional body
• Use K&R brace placement for namespaces, but put brace underneath conditions in function definition code
• ...

Answer 15

Use const identifiers by default. They provide guarantees for the reader/maintainer, and are way easier to build in than to insert afterwards.

Both member variables and methods would be declared const, as well as function arguments. const member variables enforce proper use of the initializer list.

A side-effect of this rule: avoid methods with side-effects.

Answer 16

Forbid t[i]=i++; f(i++,i);, and so on as there is no (portable) guarantees regarding what is executed first.

Answer 17

Use C++ casts instead of C casts

use:

• static_cast
• const_cast
• reinterpret_cast
• dynamic_cast

but never C-style casts.

How it clearly facilitates safer code, which minimizes the risk of enigmatic bugs, which increases maintainability, etc.

Each cast has limited powers. E.g., if you want to remove a const (for whatever reason), const_cast won't change the type at the same time (which could be a bug difficult to find).

Also, this enables a reviewer to search for them and then, the coder to justify them if needed.

Answer 18

Side note: Do not impose SESE (Single Entry Single Exit) (i.e. do not forbid more than one return, the use of break/continue/...)

In C++, this is an utopia as throw is another return point. SESE had two advantages in C and exception-less languages:

• the deterministic release of resources that is now neatly handled by the RAII idiom in C++,
• making functions easier to maintain, that should not be a concern as the functions must be kept short (as specified by the rule of "one function, one responsibility")

Answer 19

Use vector and string instead of C-style arrays and char *

Use std::vector whenever you need to create a buffer of data, even if the size is fixed.

Use std::string whenever you need to have a string.

How it clearly facilitates safer code, which minimizes the risk of enigmatic bugs, which increases maintainability, etc.?

std::vector: The user of a vector can always find its size, and the vector can be resized if needed. It can even be given (through the (&(myVector[0])) notation) to a C API. Of course, the vector will clean after itself.

std::string: Almost the same reasons above.And the fact it will always be correctly initialized, that it can't be overrun, that it will handle modifications gracefully, like concatenations, assignation, etc, and in a natural way (using operators instead of functions)

Answer 20

Never use structs without proper constructors

structs are legal C++ constructs, used to aggregate data together. Still, the data should be always properly initialized.

All C++ structs should have at least a default constructor, which will set its aggregated data to default values.

struct MyStruct // BAD
{
   int i ; bool j ; char * k ;
}

struct MyStruct // GOOD
{
   MyStruct() : i(0), j(true), k(NULL) : {}

   int i ; bool j ; char * k ;
}

And if they are usually initialized in some way, provide a constructor to enable the user to avoid a C-style struct initialization:

MyStruct oMyStruct = { 25, true, "Hello" } ; // BAD
MyStruct oMyStruct(25, true, "Hello") ;      // GOOD

How it clearly facilitates safer code, which minimizes the risk of enigmatic bugs, which increases maintainability, etc.?

Having struct without a proper constructor leaves the user of this struct the task of initializing it. So, the following code will be copy pasted from function to function:

void doSomething()
{
   MyStruct s = { 25, true, "Hello" } ;
  // Etc.
}

void doSomethingElse()
{
   MyStruct s = { 25, true, "Hello" } ;
  // Etc.
}

// Etc.

Which means that, in C++, if you need to add a field in the struct, or change the order of the internal data, you have to go through all these initializations to verify each is still correct. With a proper constructor, modifying the internals of the structs is decoupled from its use.

Answer 21

A point should be dedicated to explain the difference between value semantics and entity semantics. It could provide the typical code snippets about how copy is handled is the various cases.

See also Checklist for writing copy constuctor and assignment operator in C++

Answer 22

Public inheritance must model The Liskov Substitution Principle (LSP).

Code reuse/import without substituability must be implemented with private inheritance when a very strong coupling makes sense, or with aggregation otherwise.

Answer 23

Don't add types or functions to the global namespace.

Answer 24

Beware of C API

The C API can be very efficient, but will need exposed raw data (i.e. pointers, etc.), which won't help the safety of the code. Use existing C++ API instead, or encapsulate the C API with C++ code.

e.g.:

// char * d, * s ;
strcpy(d, s) ; // BAD

// std::string d, s ;
d = s ;        // GOOD

Never use strtok

strtok is not reentrant. Which means that if one strtok is started while another is not ended, one will corrupt the "internal data" of the other.

How it clearly facilitates safer code, which minimizes the risk of enigmatic bugs, which increases maintainability, etc.?

Using C API means using raw types, which can lead to interesting bugs like buffer overflow (and potential stack corruption) when a sprintf goes too far (or string cropping when using snprintf, which is a kind of data corruption). Even when working on raw data, malloc can be easily abused, as shown by the following code:

int * i = (int *) malloc(25) ; // Now, I BELIEVE I have an array of 25 ints!
int * j = new int[25] ;        // Now, I KNOW I have an array of 25 ints!

Etc. etc..

As for strtok: C and C++ are stack-enabled languages, that enable to user to not care about what functions are above his own on the stack, and what functions will be called below his own on the stack. strtok removes this freedom of "not caring"

Answer 25

Premature optimization is the root of all evil

Write safe and correct code first.

Then, if you have performance problems, and if your profiler told you the code is slow, you can try to optimize it.

Never believe you will optimize snippets of code better than the compiler.

When looking for optimizations, study the algorithms used, and potentially better alternatives.

How it clearly facilitates safer code, which minimizes the risk of enigmatic bugs, which increases maintainability, etc.?

Usually, "optimized" (or supposedly optimized) code is a lot less clearer, and tend to express itself through raw, near-the-machine way, instead of a more business-oriented way. Some optimizations rely of switchs, if, etc., and then will be more difficult to test because of multiple code paths.

And of course, optimization before profiling often lead to zero performance gain.

Answer 26

The best standards are those that are small and tightly focussed on what really matters to making quality code. They do not try to teach coding, they do not try to force a particular way of coding. They generally stick to consistency features and subjective reviews (eg, if the rest of your team think a piece of code is readable, fits with the consistency rules, and is commented, then its always going to be good code)

So to re-emphasise: consistency - naming convention, whitespace management, commenting blocks, directory structure. Nothing else really matters

Edit for Dustin: the big problem with standards comes with the exceptions. If you have a standard that says "1 statement per line", you cannot write the following made-up example:

SetColText(1,"col1"); SetColWidth(1, 10);
SetColText(2,"col1"); SetColWidth(2, 10);
...
SetColText(9,"col1"); SetColWidth(9, 10);

But I'd say that was more readable, and therefore less error-prone that splitting them up. (I'm sure you can come up with better examples).

This is my point - telling people how to write code, and how to format it to strict rules is always going to fall over in ways and places you didn't anticipate. So its far better to trust your coders to do it right after enforcing a few rules. If they have a few rules to follow, they will write good, disciplined code so you won't need the rest of the crappy rules.

You see some standards that go on for pages and pages. (The Philips C# one is 48 fecking pages long!)

So, given that you have a team of quality coders, what do you need to do to make it easier to work with their code? the answer is always consistency of 'where' they put the code, not how they write it. eg. you always have a bin, and obj directory in your project is a good standard. You can pick up any project and know where things are .. unlike someone building all his binaries in his c:/mybin directory because its easier for him.

Answer 27

Know who is owner of that memory.

• create objects on stack as much as possible (no useless new)
• Avoid transfer of ownership unless really needed
• Use RAII and smart pointers
• If transfer of ownership is mandated (without smart pointers), then, document clearly the code (the functions should have a non-ambiguous name, always using the same name pattern, like "char * allocateMyString()" and "void deallocateMyString(char * p)".

How it clearly facilitates safer code, which minimizes the risk of enigmatic bugs, which increases maintainability, etc.?

Not having a clear memory ownership philosophy leads to interesting bugs or memory leaks, and time lost wondering if the char * returned by this function should be deallocated by the user, or not, or given back to a special deallocation function, etc..

As much as possible, the function/object allocating the memory must be the function/object deallocating it.

Answer 28

Limit the types you use

If you need to use an integer type, choose one and keep it. This will avoid the problems associated with mixing of short, int, long, etc.. types.

// BAD
int i ;
long j ;
short k ;

// GOOD (if you choose the "int" as integer)
int i ;
int j ;
int k ;

The same goes for real types: Choose one (e.g. double), and do not use another.

Etc.

Note: There is still the issue of signed/unsigned, which can't always be avoided, and the fact STL use its own integer types (i.e. std::vector::size_type), but all the remaining code should not mixing.

Note 2: You could use typedef to "choose" your prefered type for signed integer and real numbers. This would enable a low-cost change if needed.

How it clearly facilitates safer code, which minimizes the risk of enigmatic bugs, which increases maintainability, etc.?

Some bugs are created by comparing unsigned type to signed types, mysterious loss of precision, or integer under/overflow.

Compilers usually send warnings at compile time, but then, the usually answer is to "cast" the warning away, which can help hide the error.

Edit

plinth made an useful comment I'll copy paste here:

Having written a lot of code that has to interact with things at the hardware level, I can't say much for this guideline. For this level of work, I prefer the integral types to be abstracted to names that include the precision (ie, int16, uint16, int32, uint32, etc.) – plinth Aug 18 at 20:50

plinth is right, of course. Sometimes you have to deal with int16, uint8 and other "precisely defined" types.

This does not invalidate the post above, only complete it.

The source of the bug is mixing different types (converting unsigned char into int, for example), thus, this kind of mixing must be avoided. The following rules thus apply:

• Choose one generic integral type (e.g. int), and stick to it when dealing with generic integers (the same can be said about reals)
• If (and only if) you need exact types (like uint8 or int16), use them
• Never mix different types.
• If you really must mix different types, then be very very cautious.

Below is an example of code that would break:

void * doAllocate(uint32 i)
{
   // try to allocate an array of "i" integers and returns it
}

void doSomething()
{
   uint32 i0 = 225 ;
   int8   i1 = 225 ;  // Oops...

   doAllocate(i0) ;   // This will try to allocate 255 integers
   doAllocate(i1) ;   // This will TRY TO allocate 4294967265
                      // integers, NOT 225
}

Answer 29

Principle of least surprise.

Maybe it's not the "flavor" of rules you are looking for, but I'd definitely put it first.

It is not only the root, reason and sanity check for all the boring stuff like formatting and commenting guidelines, but - to me more importantly - puts the emphasis on the code being read and understood, rather than just compiled.

It also covers the only reasonable code quality measure I have ever encountered - WTF's per minute.

I'd use that first point to stress the importance and value of clear, consistent code, and to motivate the following items in the coding standard.

Answer 30

Prefer standard-compliant code. Prefer to use the standard libraries.

Answer 31

I think a coding standard document isn't the solution to this problem. The solution is to motivate your labor to learn/care about the human side of coding - "code for people first and computers last".

Obviously it is not possible to just fire the ones that don't care - but a standard document isn't going to help them much, either.

Answer 32

I suggest just requiring developers to read a bunch of guidelines, and the effective C++ and more effective C++ books by Meyers.

If you want lightweight, you are going to have to rely on common sense and a common ideal.

Code reviews help enforce this as well.

To keep it lightweight I would avoid a document and code police. Praise good code publicly.

EDIT - I started with a comment here, but will put it in the response for ease of viewing:

reviews done correctly will do wonders - but you can't allow reporting hierarchies into the review and no statistics with people's names can be on the review results.

Make sure to keep the document small and be sure to give REASONS for the "rule"/guideline. Without that then you ae just demanding blind obedience. With rationale and reasons you educate so that actually posting/writing the "rule" becomes unneeded. (as the concept will be internalized)

Answer 33

Always, always, always do proper data member initialization on object construction.

I ran into a problem where an object constructor was relying on some "default" initialization for its data members. Building the code under two platforms (Windows/Linux) gave different results and a hard-to-find memory bug. The result was that a data member was not initialized in the constructor, and used before it was initialized. On one platform (Linux), the compiler initialized it to what the code writer thought appropriate default. On Windows, the value was initialized to something - but garbage. On use of the data member, everything went haywire. Once the initialization was fixed - no more problem.

Answer 34

Probably a no-brainer, but nevertheless an important rule:

Avoid undefined behavior.

There's an awful lot of it in C++, and it's probably impossible to write a nontrivial application that doesn't depend on it somehow, but the general rule should still be "undefined behavior is bad". (Because sadly there are C++ programmers out there who feels that "it works on my machine/compiler" is good enough).

If you have to rely on it, make it clear to everyone what, why, where and how.

Answer 35

Make sure destructors are defined as virtual:

 class GoodClass {
 public:
   GoodClass();
   virtual ~GoodClass()
 };

 class BadClass {
 public:
   BadClass();
   ~BadClass()
 };

Answer 36

The Art of Computer Programming Tome{1,2,3}

Answer 37

Pass input arguments by const reference, and output or input-output arguments by pointer. This is a rule borrwed from the Google style guide.

I used to have an absolute aversion to pointers and preferred to use references whenever possible (as suggested by one of the posters in this thread). However, adopting this output-arg-as-pointer convention has made my functions much more readable. For example,

SolveLinearSystem(left_hand_side, right_hand_side, &params);

makes it clear that "params" is being written to.