What C++ pitfalls should I avoid?

I remember first learning about vectors in the STL and after some time, I wanted to use a vector of bools for one of my projects. After seeing some strange behavior and doing some research, I learned that a vector of bools is not really a vector of bools.

Are there any other common pitfalls to avoid in C++?

A short list might be:

• Avoid memory leaks through use shared pointers to manage memory allocation and cleanup
• Use the Resource Acquisition Is Initialization (RAII) idiom to manage resource cleanup - especially in the presence of exceptions
• Avoid calling virtual functions in constructors
• Employ minimalist coding techniques where possible - for example, declaring variables only when needed, scoping variables, and early-out design where possible.
• Truly understand the exception handling in your code - both with regard to exceptions you throw, as well as ones thrown by classes you may be using indirectly. This is especially important in the presence of templates.

RAII, shared pointers and minimalist coding are of course not specific to C++, but they help avoid problems that do frequently crop up when developing in the language.

Some excellent books on this subject are:

• Effective C++ - Scott Meyers
• More Effective C++ - Scott Meyers
• C++ Coding Standards - Sutter & Alexandrescu
• C++ FAQs - Cline

Reading these books has helped me more than anything else to avoid the kind of pitfalls you are asking about.

Pitfalls in decreasing order of their importance

First of all, you should visit the award winning C++ FAQ. It has many good answers to pitfalls. If you have further questions, visit ##c++ on irc.freenode.org in IRC. We are glad to help you, if we can. Note all the following pitfalls are originally written. They are not just copied from random sources.

---

> delete[] on new, delete on new[]

Solution: Doing the above yields to undefined behavior: Everything could happen. Understand your code and what it does, and always delete[] what you new[], and delete what you new, then that won't happen.

Exception:

typedef T type[N]; T * pT = new type; delete[] pT;

You need to delete[] even though you new, since you new'ed an array. So if you are working with typedef, take special care.

---

> Calling a virtual function in a constructor or destructor

Solution: Calling a virtual function won't call the overriding functions in the derived classes. Calling a pure virtual function in a constructor or desctructor is undefined behavior.

---

> Calling delete or delete[] on an already deleted pointer

Solution: Assign 0 to every pointer you delete. Calling delete or delete[] on a null-pointer does nothing.

---

> Taking the sizeof of a pointer, when the number of elements of an 'array' is to be calculated.

Solution: Pass the number of elements alongside the pointer when you need to pass an array as a pointer into a function. Use the function proposed here if you take the sizeof of an array that is supposed to be really an array.

---

> Using an array as if it were a pointer. Thus, using T ** for a two dimentional array.

Solution: See here for why they are different and how you handle them.

---

> Writing to a string literal: char * c = "hello"; *c = 'B';

Solution: Allocate an array that is initialized from the data of the string literal, then you can write to it:

char c[] = "hello"; *c = 'B';

Writing to a string literal is undefined behavior. Anyway, the above conversion from a string literal to char * is deprecated. So compilers will probably warn if you increase the warning level.

---

> Creating resources, then forgetting to free them when something throws.

Solution: Use smart pointers like std::unique_ptr or std::shared_ptr as pointed out by other answers.

---

> Modifying an object twice like in this example: i = ++i;

Solution: The above was supposed to assign to i the value of i+1. But what it does is not defined. Instead of incrementing i and assigning the result, it changes i on the right side as well. Changing an object between two sequence points is undefined behavior. Sequence points include ||, &&, comma-operator, semicolon and entering a function (non exhaustive list!). Change the code to the following to make it behave correctly: i = i + 1;

---

Misc Issues

> Forgetting to flush streams before calling a blocking function like sleep.

Solution: Flush the stream by streaming either std::endl instead of \n or by calling stream.flush();.

---

> Declaring a function instead of a variable.

Solution: The issue arises because the compiler interprets for example

Type t(other_type(value));

as a function declaration of a function t returning Type and having a parameter of type other_type which is called value. You solve it by putting parentheses around the first argument. Now you get a variable t of type Type:

Type t((other_type(value)));

---

> Calling the function of a free object that is only declared in the current translation unit (.cpp file).

Solution: The standard doesn't define the order of creation of free objects (at namespace scope) defined across different translation units. Calling a member function on an object not yet constructed is undefined behavior. You can define the following function in the object's translation unit instead and call it from other ones:

House & getTheHouse() { static House h; return h; }

That would create the object on demand and leave you with a fully constructed object at the time you call functions on it.

---

> Defining a template in a .cpp file, while it's used in a different .cpp file.

Solution: Almost always you will get errors like undefined reference to .... Put all the template definitions in a header, so that when the compiler is using them, it can already produce the code needed.

---

> static_cast<Derived*>(base); if base is a pointer to a virtual base class of Derived.

Solution: A virtual base class is a base which occurs only once, even if it is inherited more than once by different classes indirectly in an inheritance tree. Doing the above is not allowed by the Standard. Use dynamic_cast to do that, and make sure your base class is polymorphic.

---

> dynamic_cast<Derived*>(ptr_to_base); if base is non-polymorphic

Solution: The standard doesn't allow a downcast of a pointer or reference when the object passed is not polymorphic. It or one of its base classes has to have a virtual function.

---

> Making your function accept T const **

Solution: You might think that's safer than using T **, but actually it will cause headache to people that want to pass T**: The standard doesn't allow it. It gives a neat example of why it is disallowed:

int main() {
    char const c = ’c’;
    char* pc;
    char const** pcc = &pc; //1: not allowed
    *pcc = &c;
    *pc = ’C’; //2: modifies a const object
}

Always accept T const* const*; instead.

Another (closed) pitfalls thread about C++, so people looking for them will find them, is Stack Overflow question C++ pitfalls.

Some must have C++ books that will help you avoid common C++ pitfalls:

Effective C++
More Effective C++
Effective STL

The Effective STL book explains the vector of bools issue :)

Brian has a great list: I'd add "Always mark single argument constructors explicit (except in those rare cases you want automatic casting)."

The web page C++ Pitfalls by Scott Wheeler covers some of the main C++ pitfalls.

Not really a specific tip, but a general guideline: check your sources. C++ is an old language, and it has changed a lot over the years. Best practices have changed with it, but unfortunately there's still a lot of old information out there. There have been some very good book recommendations on here - I can second buying every one of Scott Meyers C++ books. Become familiar with Boost and with the coding styles used in Boost - the people involved with that project are on the cutting edge of C++ design.

Do not reinvent the wheel. Become familiar with the STL and Boost, and use their facilities whenever possible rolling your own. In particular, use STL strings and collections unless you have a very, very good reason not to. Get to know auto_ptr and the Boost smart pointers library very well, understand under which circumstances each type of smart pointer is intended to be used, and then use smart pointers everywhere you might otherwise have used raw pointers. Your code will be just as efficient and a lot less prone to memory leaks.

Use static_cast, dynamic_cast, const_cast, and reinterpret_cast instead of C-style casts. Unlike C-style casts they will let you know if you are really asking for a different type of cast than you think you are asking for. And they stand out viisually, alerting the reader that a cast is taking place.

I've already mentioned it a few times, but Scott Meyers' books Effective C++ and Effective STL are really worth their weight in gold for helping with C++.

Come to think of it, Steven Dewhurst's C++ Gotchas is also an excellent "from the trenches" resource. His item on rolling your own exceptions and how they should be constructed really helped me in one project.

Two gotchas that I wish I hadn't learned the hard way:

(1) A lot of output (such as printf) is buffered by default. If you're debugging crashing code, and you're using buffered debug statements, the last output you see may not really be the last print statement encountered in the code. The solution is to flush the buffer after each debug print (or turn off the buffering altogether).

(2) Be careful with initializations - (a) avoid class instances as globals / statics; and (b) try to initialize all your member variables to some safe value in a ctor, even if it's a trivial value such as NULL for pointers.

Reasoning: the ordering of global object initialization is not guaranteed (globals includes static variables), so you may end up with code that seems to fail nondeterministically since it depends on object X being initialized before object Y. If you don't explicitly initialize a primitive-type variable, such as a member bool or enum of a class, you'll end up with different values in surprising situations -- again, the behavior can seem very nondeterministic.

Using C++ like C. Having a create-and-release cycle in the code.

In C++, this is not exception safe and thus the release may not be executed. In C++, we use RAII to solve this problem.

All resources that have a manual create and release should be wrapped in an object so these actions are done in the constructor/destructor.

// C Code
void myFunc()
{
    Plop*   plop = createMyPlopResource();

    // Use the plop

    releaseMyPlopResource(plop);
}

In C++, this should be wrapped in an object:

// C++
class PlopResource
{
    public:
        PlopResource()
        {
            mPlop=createMyPlopResource();
            // handle exceptions and errors.
        }
        ~PlopResource()
        {
             releaseMyPlopResource(mPlop);
        }
    private:
        Plop*  mPlop;
 };

void myFunc()
{
    PlopResource  plop;

    // Use the plop
    // Exception safe release on exit.
}

The book C++ Gotchas may prove useful.

Here are a few pits I had the misfortune to fall into. All these have good reasons which I only understood after being bitten by behaviour that surprised me.

• virtual functions in constructors [aren't](https://stackoverflow.com/questions/36832 "Virtual functions in constructors, why do languages differ?").

• Don't violate the ODR (One Definition Rule), that's what anonymous namespaces are for (among other things).

• Order of initialization of members depends on the order in which they are declared.

    class bar {
        vector<int> vec_;
        unsigned size_; // Note size_ declared *after* vec_
    public:
        bar(unsigned size)
            : size_(size)
            , vec_(size_) // size_ is uninitialized
            {}
    };
  

• Default values and virtual have different semantics.

    class base {
    public:
        virtual foo(int i = 42) { cout << "base " << i; }
    };
  
    class derived : public base {
    public:
        virtual foo(int i = 12) { cout << "derived "<< i; }
    };
  
    derived d;
    base& b = d;
    b.foo(); // Outputs `derived 42`
  

The most important pitfalls for beginning developers is to avoid confusion between C and C++. C++ should never be treated as a mere better C or C with classes because this prunes its power and can make it even dangerous (especially when using memory as in C).

Check out boost.org. It provides a lot of additional functionality, especially their smart pointer implementations.

PRQA have an excellent and free C++ coding standard based on books from Scott Meyers, Bjarne Stroustrop and Herb Sutter. It brings all this information together in one document.

1. Not reading the C++ FAQ Lite. It explains many bad (and good!) practices.
2. Not using Boost. You'll save yourself a lot of frustration by taking advantage of Boost where possible.

Be careful when using smart pointers and container classes.

Avoid pseudo classes and quasi classes... Overdesign basically.

Forgetting to define a base class destructor virtual. This means that calling delete on a Base* won't end up destructing the derived part.

Read the book C++ Gotchas: Avoiding Common Problems in Coding and Design.

Keep the name spaces straight (including struct, class, namespace, and using). That's my number-one frustration when the program just doesn't compile.

To mess up, use straight pointers a lot. Instead, use RAII for almost anything, making sure of course that you use the right smart pointers. If you write "delete" anywhere outside a handle or pointer-type class, you're very likely doing it wrong.

• Blizpasta. That's a huge one I see a lot...

• Uninitialized variables are a huge mistake that students of mine make. A lot of Java folks forget that just saying "int counter" doesn't set counter to 0. Since you have to define variables in the h file (and initialize them in the constructor/setup of an object), it's easy to forget.

• Off-by-one errors on for loops / array access.

• Not properly cleaning object code when voodoo starts.

> * static_cast downcast on a virtual base class

Not really... Now about my misconception: I thought that A in the following was a virtual base class when in fact it's not; it's, according to 10.3.1, a polymorphic class. Using static_cast here seems to be fine.

struct B { virtual ~B() {} };

struct D : B { };

In summary, yes, this is a dangerous pitfall.

Always check a pointer before you dereference it. In C, you could usually count on a crash at the point where you dereference a bad pointer; in C++, you can create an invalid reference which will crash at a spot far removed from the source of the problem.

class SomeClass
{
    ...
    void DoSomething()
    {
        ++counter;    // crash here!
    }
    int counter;
};

void Foo(SomeClass & ref)
{
    ...
    ref.DoSomething();    // if DoSomething is virtual, you might crash here
    ...
}

void Bar(SomeClass * ptr)
{
    Foo(*ptr);    // if ptr is NULL, you have created an invalid reference
                  // which probably WILL NOT crash here
}

Intention is (x == 10):

if (x = 10) {
    //Do something
}

I thought I would never make this mistake myself, but I actually did it recently.

The essay/article Pointers, references and Values is very useful. It talks avoid avoiding pitfalls and good practices. You can browse the whole site too, which contains programming tips, mainly for C++.

I spent many years doing C++ development. I wrote a quick summary of problems I had with it years ago. Standards-compliant compilers are not really a problem anymore, but I suspect the other pitfalls outlined are still valid.

Forgetting an & and thereby creating a copy instead of a reference.

This happened to me twice in different ways:

• One instance was in an argument list, which caused a large object to be put on the stack with the result of a stack overflow and crash of the embedded system.

• I forgot the & on an instance variable, with the effect that the object was copied. After registering as a listener to the copy I wondered why I never got the callbacks from the original object.

Both where rather hard to spot, because the difference is small and hard to see, and otherwise objects and references are used syntactically in the same way.

#include <boost/shared_ptr.hpp>
class A {
public:
  void nuke() {
     boost::shared_ptr<A> (this);
  }
};

int main(int argc, char** argv) {
  A a;
  a.nuke();
  return(0);
}