31

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.

6
  • It is likely that here you will create an incohesive collection of rules, most of which won't address problems resulting in undefined behaviour. Oct 28, 2008 at 12:25
  • 1
    An alternative approach, is to take an existing coding standard (eg. AV JSF, MISRA C++, www.codingstandard.com) and then post the rules you feel are good candidates. Oct 28, 2008 at 12:33
  • how can style be worth a down-vote. You ask for best practices in standards, then insist on ignoring the best. Code where everyone chooses their own naming convention (for example) is the worst readable possible. Vote how you like, brothers!
    – gbjbaanb
    Oct 28, 2008 at 13:37
  • @gbjbaanb: Please try to understand the purpose here. Style is important, but if you were to pick the (let's say) 10 most important things to have in your light-weight easy-reviewable standard in absense of any process whatsovever, - then style should have pretty low priority.
    – sharkin
    Oct 28, 2008 at 13:59
  • I like the way that currently the top two options "keep functions under 25 lines", and "sprinkle asserts all over the place" are nigh-on mutually contradictory. If you want to do both, buy a taller monitor ;-) Oct 28, 2008 at 14:02

40 Answers 40

68

Prefer RAII.

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

6
  • i would say "prefer RAII" - i certainly wouldn't require it in a standard. Oct 28, 2008 at 11:47
  • Agreed. One place I worked people would argue symantics like "perfer". Eventually our standard was written like everything was an absolute requirement (dispite being able to deviate from it after a review).
    – Aardvark
    Oct 28, 2008 at 12:52
  • Without RAII, it's quite difficult to write error-safe code that's easy to maintain. Oct 28, 2008 at 13:05
  • 2
    I wasn't even aware of the term "RAII" until recently. I always considered the concept as "common sense".
    – Aardvark
    Oct 28, 2008 at 13:35
  • 2
    It amazes me how many C++ programmers don't know the RAII idiom. I've had to enforce this with a rule saying "Never use 'new' without checking with the team lead." Oct 28, 2008 at 20:09
58

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.

------ EDIT

const member variables seemed like a good idea; this is not always the case: they effectively make any object immutable. This becomes problematic for e.g. sorting a vector.

struct A {
    const int i;
};

bool operator<(const A& lhs, const A& rhs) {
    return lhs.i < rhs.i;
}

int main() {
    std::vector<A> as;
    as.emplace_back(A{1});
    std::sort(begin(as), end(as));
}

error message:

... note: copy assignment operator of 'A' is implicitly deleted because
field 'i' is of const-qualified type 'const int'
...
in instantiation of function template specialization 'std::sort<...>'
requested here

    std::sort(begin(as), end(as));

4
  • So easy to use, and so much more safety in the code. +1. In fact, this is more important than the amusing "use if(0 == p) instead of if(p == 0)"...
    – paercebal
    Oct 28, 2008 at 13:08
  • Indeed, have p const, and you won't have to worry about if (p=0). Oct 28, 2008 at 13:41
  • Voted up, but I'd suggest making it clearer that you are talking about methods as well as just data items.
    – T.E.D.
    Oct 28, 2008 at 13:56
  • 1
    Good point about inserting const later - that's painful... +1
    – Aardvark
    Oct 28, 2008 at 14:59
48

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.

4
  • Moreover a C cast is always handled as a reinterpret_cast<> when dealing with forward declarations. And C casts is unable to correctly downcast or crosscast. Oct 28, 2008 at 13:16
  • 1
    Yes, absolutely, but I don't know that I would recommend reinterpret_cast either... Aug 18, 2009 at 21:11
  • This conflicts with the "use weakest possible cast" rule, as reinterpret_cast is stronger than C-style cast and sometimes you have nothing between it and static_cast other than c-style cast. For instance, converting a pointer to a byte buffer to a struct{} (regular binary parsing pattern). Would you recommend a reinterpret_cast? I think not. Jan 4, 2010 at 9:52
  • 2
    @PavelRadzivilovsky, this is exactly one of the cases for reinterpret_cast<>. reinterpret_cast<> is designed for converting between Foo* and Bar*, where Foo and Bar are completely different types (in your example, std::uint8* and struct foo. reinterpret_cast<> is also weaker than a C-style cast, because it cannot cast away const or volatile Oct 14, 2012 at 17:58
39

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

5
  • 2
    But if you dereference a pointer to initialize the reference, make sure the pointer isn't NULL first! Oct 28, 2008 at 14:33
  • Yes - but if you always used references you wouldn't have that pointer to begin with! I'm kidding... I've been bitten by the very issue you bring up many times.
    – Aardvark
    Oct 28, 2008 at 14:58
  • 2
    +1 to the effect, not to the reasoning. i sometimes return null iterator, e.g. find(). is that bad? higher level languages that use reference types typically have a None object. Oct 28, 2008 at 21:26
  • @Dustin Getz - so what reasoning would you like to see? I'll edit.
    – Aardvark
    Oct 28, 2008 at 23:40
  • 1
    @Dustin, @Aardvark: The problem is that if you return a reference then you're kind of forced to use exceptions to indicate failure rather than testing a returned pointer for NULL. However, if you don't need the error checking, use a reference because it prevents a lot of stupid mistakes. It also makes code a little easier to read. I'd also advocate using references to simulate Pascal's with, e.g. SubClassType &sub = SomePtr->SomeLowerLevel[index]->member; and then you use sub.
    – Harvey
    Jan 10, 2011 at 21:21
36

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.

5
  • 4
    ... and compile with WISE (Warning is Error)
    – xtofl
    Oct 28, 2008 at 10:52
  • 1
    Another way: If p is const, then if(p = 0) won't compile anyway, but this is another post...
    – paercebal
    Oct 28, 2008 at 13:35
  • For moderation, see stackoverflow.com/questions/237719 I am surprised myself and not happy about how much of a nerve it hit. Nonetheless, I think it is a valuabe rule!
    – peterchen
    Oct 28, 2008 at 14:43
  • 1
    Unfortunately, MSVC's C++ libraries have many of errors with /W4, and hundreds at /Wall Oct 26, 2011 at 16:54
  • 1
    @Mooing Duck: I've run into that problem. Fortunately, it's solvable: stackoverflow.com/questions/525677/…
    – Ferruccio
    Oct 26, 2011 at 17:18
34

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)

7
  • 1
    The problem with this is that I write a lot of OS-interface code, which makes heavy use of char arrays. Having some kind of standard that required me to use std::string for objects that are never accessed as anything but char arrays would lead to a lot of dumb code.
    – T.E.D.
    Oct 28, 2008 at 13:43
  • The solution, then, would be to interface the OS API with C++ friendly methods. I worked on such thing to interface my C++ code with raw Win32 API, and usually, it is Ok. And my C++ interface checked for "GetLastError" and logged the info, which no developer did on its own!
    – paercebal
    Oct 28, 2008 at 14:00
  • for domain specific solutions (drivers and such), of course you can prefer native APIs, so long as you can justify it. Oct 28, 2008 at 19:23
  • @Dustin Getz: Of course, you're right. Drivers are low level APIs themselves, the constraints are very specifics, so the solution usually are, too
    – paercebal
    Oct 30, 2008 at 21:34
  • 1
    What is std::string? Maybe you meant std::wstring? :) Aug 18, 2009 at 10:42
32

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.

2
  • It's indeed important. And it is a consequence of : "a function must do only one thing, and do it well" Oct 28, 2008 at 12:59
  • I've worked on jobs that had McCabe complexity limits for all functions. The cutoff was kind of arbitrary some times, but the overall effect forced smaller routines, which was great.
    – T.E.D.
    Oct 28, 2008 at 13:59
31

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
3
  • I would even add: compiletime assert where possible.
    – xtofl
    Oct 28, 2008 at 10:51
  • Regarding Programming by Contract, see also B.Meyer book, and M.Wilson's Imperfect C++. Oct 28, 2008 at 13:28
  • I would even add: release-time check at nearly all places where you have debug-time assert(). The only reason to have one without the other is performance, which is seldom a resonable subject for optimization. Jan 4, 2010 at 9:54
29

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.

1
  • I'd add that the "owner" policy should also include a "transfer of ownership" policy.
    – twokats
    Nov 5, 2008 at 21:04
27

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")
4
  • I disagree that single point of return ever made functions easier to maintain. It usually just complicates the function body unnecessarily. Oct 28, 2008 at 13:20
  • Greg, I totally agree with you. I don't think a SESE function is easier to read. Some think so, but it is not my case either. Oct 28, 2008 at 13:22
  • Modding up, but I generally don't mind such requirements, because there is almost never more than one exit point from a routine or loop. There might be more than one place the exit came from, but never more than one point it goes to. Without goto's all routines are single exit point.
    – T.E.D.
    Oct 28, 2008 at 13:51
  • ..another way to look at this is that if you have exceptions, all your routines and loops are multiple exit anyway. :-)
    – T.E.D.
    Oct 28, 2008 at 13:53
26

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.

24

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;
}
7
  • 7
    shrug, i think its subjective. for early bailout type checks the braces just get in the way. i'd be annoyed if this was in a coding standard. Oct 28, 2008 at 21:28
  • 4
    the indentation is more important than the braces - that's what shows you where something should be, the "++i;" will always stand out in well-indented code.
    – gbjbaanb
    Nov 1, 2008 at 16:04
  • 2
    I agree. I see no advantage in leaving out the braces, only potential problems. Aug 18, 2009 at 21:20
  • 2
    -1 shortening code is important. Proper indentation seems to solve these problems, and misindentation problems are easy to spot during CR. Jan 4, 2010 at 9:56
  • 1
    this is subjective, but in my book avoiding potential mistakes is more important than having less curly braces to look at
    – slf
    Mar 23, 2010 at 13:26
15

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

1
  • Too bad this item is justified. Too many out there that justify their code by the only mean of "works for me" ... Jun 20, 2011 at 12:03
12

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);
7
  • 4
    I agree with the spirit of this rule, but I think the non-trivial example you give is perfectly fine. I would disallow nested ternary operators.
    – Ferruccio
    Oct 28, 2008 at 10:48
  • The ternary operator should be reserved to the cases where the result is more important than the test. BTW, it quite useful to use const pro-activelly. Oct 28, 2008 at 13:01
  • Really, the second example violates my rule of only one thing per line. An assignment with the operator? would make this code readable. In shorter terms, this is'nt an example of bad operator? use.
    – kenny
    Oct 28, 2008 at 14:48
  • This shouldn't be in a coding standard because there's no objective definition of good/bad. It'd be covered in an introductory statement "Write clear, maintainable code." Aug 18, 2009 at 10:48
  • I agree with Ferruccio. That example is ok, it is the nested form that gives me headaches. Aug 18, 2009 at 21:21
9

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.

2
  • 1
    I advocate exactly this in my company. Lint should always be used, and will catch this problem. Plus, any experienced dev won't make this mistake anymore (not even a typo). Lastly, visually, the style with the constant first is hard on my eyes... so kudos, I agree.
    – Dan
    Oct 28, 2008 at 13:28
  • 1
    Placing the constant first harms readability by diffusing the flow of information. I absolutely agree with letting the tools do their job here. Aug 18, 2009 at 10:49
9

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.

12
  • The interesting part about this answer is that I disagree with the title, but very strongly agree with the text.
    – T.E.D.
    Oct 28, 2008 at 13:40
  • You're right: I'll change the title...
    – paercebal
    Oct 28, 2008 at 13:42
  • 5
    A constructor makes the type a non-POD. How do you create a POD struct?
    – MSalters
    Oct 28, 2008 at 14:50
  • @MSalters: Good question, the answer being already given above, but not clearly, it's true: In C++, avoid POD structs as much as possible...
    – paercebal
    Oct 28, 2008 at 15:02
  • 1
    If you are simply initializing the values of i and k to defaults for the type, you should use i(), k() instead. That way the items in the initializer list that have particular meaning to that constructor are the only ones with specific values in parentheses. Aug 18, 2009 at 10:56
8

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

1
  • 1
    This is a good suggestion, but the rationale is missing: you pollute the global namespace by doing it and thereby potentially causing a whole lot problems if the name exists elsewhere.
    – twokats
    Nov 5, 2008 at 21:06
8

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.

6

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

5
  • I would have to look it up, but I can hardly believe the Standard leaves this construct undetermined.
    – xtofl
    Oct 28, 2008 at 19:54
  • 4
    "Undefined" is the word. What specifiy how the instructions are sequenced are the Sequence points, and there is no sequence point between the two evaluations of i and i++ in both examples. This is actually a quite well known issue in C&C++. See the FAQ C++ lite §39.15 and §39.16 Oct 28, 2008 at 20:46
  • Again, this shouldn't be in a written coding standard because it just clutters up the document with obvious information. Aug 18, 2009 at 10:51
  • Unfortunately this is not that obvious for everybody. Still your comment is interresting. It raises a new question: where does the obvious starts and where does it ends? Because in the end, a standard is about safety rules that are well known and understood by a few people. Aug 18, 2009 at 15:31
  • What? you might as well forbid my favorite: const double PI = 3.141592653658979; *(double*)&PI = 4; Seriously, if you're going to talk about actual errors being disallowed in a style document, the document would be infinitely long!
    – Dov
    Jun 19, 2011 at 2:06
5

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.

4

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.

4

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.

6
  • Very interesting. Please edit and elaborate a bit more on why this is so.
    – sharkin
    Oct 28, 2008 at 10:20
  • domain specific argument. if you're not using exceptions, you need to understand why they are awesome, have metrics documenting why they aren't 'whatever' enough and how much more 'whatever' they need to be, and be really upset that you can't use them. Only then should you stop using them. Oct 28, 2008 at 10:54
  • There are very specific reasons for not using exceptions, however, EC++ is not the definitive source I would use. EC++ bans lots of features from C++ and actually caused the ISO C++ Committee to respond to the claims that were made by the standard. Oct 28, 2008 at 12:31
  • The only valid reason I've read to not use exceptions is when we are dealing with a legacy code that does nothing about exceptions -- see google C++ Coding Standard. Oct 28, 2008 at 13:03
  • Richar Corden is right. Just reading en.wikipedia.org/wiki/Embedded_C%2B%2B , and that they removed namespaces, templates and C++ casts show AC++ was a joke more than anything else
    – paercebal
    Oct 28, 2008 at 13:06
4

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.

4

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"

4

Avoid using generated copy constructor and operator= by default.

  • If you want your object to be copiable.
    • If every attribute can be trivially copied, comment clearly you are using implicit copy constructor and operator= deliberately.
    • Otherwise, write your own constructors, using the initialization field to initialize attributes and following the header order (which is the real construction order).
  • If still don't know (default option) or you think you dont want to copy the objects of a certain class, declare its copy constructor and operator= as private. This way the compiler will let you know when you are doing something you don't want to do.
    class foo
    {
       //...
    private:
       foo( const foo& );
       const foo& operator=( const foo& );
    };

Or in a cleaner way if you are using boost:

    class foo : private boost::noncopyable
    {
      ...
    };
3
  • This rule is two simplistic. When all the attributes are copyable and assignable, AND when we want the new class to be copyable and assignalble, we must prefer the default generated copy-constructor and assignement operator. In those cases, we should consider leaving a comment stating that this is a deliberate choice as all the members are copyable and assignable. We are back to my answer: stackoverflow.com/questions/242728/… Aug 18, 2009 at 15:26
  • Yes, if the commentary is clear enough to prevent anybody to add a new non trivial-copy attribute to the class, breaking the initial assumptions. Aug 18, 2009 at 20:45
  • Edited to clarify this rule is tells you that you should avoid using them by default, and make the non-copyable behavior the default option. Aug 18, 2009 at 20:58
2

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
  • ...
2

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

2

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.

18
  • I think I have developed many non-trivial C++ applications that don't depend on undefined behaviour
    – Elemental
    Oct 24, 2009 at 9:50
  • I think you just don't realize that they depend on undefined behavior. ;)
    – jalf
    Oct 24, 2009 at 21:05
  • I think I have developed many non-trivial C++ applications that don't depend on undefined behaviour, and I am pretty certain of it. If you know C++ very well, it's a very, very tangible task ;) Jun 20, 2011 at 9:43
  • @phresnel: were all your apps singlethreaded? Multithreading is undefined behavior in C++03. Just an example ;)
    – jalf
    Jun 20, 2011 at 10:50
  • ?? Whether multithreading is undefined behaviour or not solely depends on how the multithreading library is implemented. "Multithreading is not defined in C++2003" is completely different from "Multithreading is undefined behaviour". The set phrase and meaning of "undefined behaviour" is well defined in the C++ standard, it is not some colloquial term. Jun 20, 2011 at 11:22
2

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.

1

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.

4
  • I like this better than "always use curly braces."
    – Ferruccio
    Oct 28, 2008 at 10:36
  • 'not always' is one of the choices that burdens the brain understanding the code. 'always' leads to 1. less choice=>less time spent choosing and 2. less occasion to write bugs overlooked by indentation (especially for people who also write python...)
    – xtofl
    Oct 28, 2008 at 10:50
  • format for readability with braces or whitespace, but i certainly think requiring them is arbitrary. Oct 28, 2008 at 11:27
  • @xtofl: even if you're supposed to always do it one way, the compiler takes more than that, so ignorant cases are inevitable. i'd argue brainparsing braces doesn't burden the brain - it becomes rote. Oct 28, 2008 at 11:30

Not the answer you're looking for? Browse other questions tagged or ask your own question.