tags:

views:

4070

answers:

30

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.

Anyone out there have any other common pitfalls to avoid in C++?

+3  A: 

This book may prove useful: http://www.semantics.org/cpp_gotchas/

Matt Rogish
+7  A: 

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

sparkes
+15  A: 

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 :)

17 of 26
+59  A: 

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, 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.

Brian Stewart
you have specified the correct and best books that i was looking for. :)
mahesh
"Avoid calling virtual functions in constructors" <-- I would upgrade that one from "Avoid" to "Never". +1 though. (Namely because it's undefined behavior)
Billy ONeal
Maybe include virtual destructors and how to catch (and rethrow) exceptions correctly?
Asgeir S. Nilsen
@BillyONeal i would probably leave it to "avoid". But in any case, behavior is well defined for virtual calls in constructors. Such a call is not undefined behavior, except when the call happens to a pure virtual function from within a pure virtual class' constructor (and analogous for destructors)
Johannes Schaub - litb
+3  A: 

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).

Konrad Rudolph
+4  A: 

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

Paul Whitehurst
+6  A: 

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.

Rob Wells
+6  A: 

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.

Tyler
the solution is not to debug with prints
Dustin Getz
Sometimes that is the only option... for example debugging crashes which happen only in Release code and/or on a target architecutre / platform different from which you are developing on.
xan
There are definitely more sophisticated ways to debug. But using prints is tried and true, and works in a lot more places than you might have access to a nice debugger. I'm not the only one who thinks so - see Pike and Kernighan's Practice of Programming book, for one.
Tyler
+1 for noting the nondeterministic initialisation of global objects. (There are some rules, but they're not as intuitive or complete as we'd like.)
j_random_hacker
printf (and std::cout) is (are) often only line buffered, so as long as you are relatively certain you're not crashing between starting the printf and hitting the newline, you should be ok. Consider also compiler bugs that prevent the generation of debug symbols <grumble grumble>
dash-tom-bang
+1  A: 

PRQA have an excellent C++ coding standard based on books from Scott Meyers, Bjarne Stroustrop and Herb Sutter. It brings all this information together in one document. You can download a free copy from http://www.codingstandard.com/.

cdv
+4  A: 

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.

  • 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`
    
Motti
That last one's a tricky one! Ouch!
j_random_hacker
C# does the same thing (as the virtual/default value one), now that C# 4 has default values.
BlueRaja - Danny Pflughoeft
+2  A: 

Be careful when using smart pointers and container classes.

Registered User
Question for answer: What's wrong with using smart pointers with container classes? ex: vector<shared_ptr<int> >. Can you elaborate?
Aaron
he's referring to containers of auto_ptr which is forbidden but sometimes compiles
Dustin Getz
@Aaron: Specifically, auto_ptr's assignment operator destroys its source operand, meaning it can't be used with standard containers which rely on this not happening. shared_ptr is fine however.
j_random_hacker
+12  A: 

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

0124816
+7  A: 

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.

Avdi
+2  A: 

Avoid Pseudo-Classes and Quasi-Classes...overdesign basically.

epatel
I am currently working on such a project with (n) quasiclasses. We need more awareness of this anti-pattern!
DarenW
+3  A: 
  1. Not reading C++ FAQ Lite. It explains many bad (and good!) practices.
  2. Not Using Boost (tm). You'll save yourself a lot of frustration by taking advantage of Boost where possible.
Eric P. Mangold
+6  A: 

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.
}
Martin York
i'm not sure whether we should add it. but maybe we should make it noncopyable/nonassignable?
Johannes Schaub - litb
A: 

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
}
Mark Ransom
Checking for NULL does not help much. A pointer may have a non-null value and still point to a deleted or otherwise invalid object.
Nemanja Trifunovic
True, but in my experience a NULL pointer is more common than the other kinds of invalid pointers. Maybe that's because I make a habit of NULLing my pointers after deleting them.
Mark Ransom
This is part of your error-handling strategy. I'ld say, avoid NULL pointer checking in the core code (rather assert), but guarantee you don't pass in invalid values (design by contract).
xtofl
A: 

Intention is (x == 10)

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

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

blizpasta
Pretty much any compiler these days will issue a warning for this
Adam Rosenfield
doing a constant == to a variable will help spot these mistakes, say if( 10 = x ), the compiler will error out on that
PiNoYBoY82
That doesn't help if you intended `if (x == y)`
dan04
+1  A: 

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

David Thornley
+1  A: 

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.

David Thornley
+1  A: 

blizpasta, that's a huge one I see alot...

uninitialized variables are a huge mistake that students of mine make. Alot of java folk 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.

A: 

This is an exact duplicate of this question.

Brian Stewart
Thanks for pointing this out.
Konrad Rudolph
+1  A: 

Edit: I've rewritten this posting. I had a wrong definition of “virtual base class” in mind when writing my original definition.


  • static_cast downcast on a virtual base class

Thanks for your edit citing the standard. Now about my misconception: I though 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. Thanks for pointing it out.

Konrad Rudolph
see my enhanced question above
Johannes Schaub - litb
A: 
#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);
}
gsarkis
Ones inability to use boost::shared_ptr is hardly a pitfall of the language.
0xC0DEFACE
+1. Though the shared_ptr docs state that this usage is not supported (and provide a workaround, enable_shared_from_this), this is a common use-case, and it's not immediately obvious that the above code will fail. It even appears to play by the rule of "immediately wrap any raw pointer in a shared_ptr". A genuine pitfall IMHO.
j_random_hacker
+2  A: 

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

xtofl
+24  A: 

Pitfalls in decreasing order of their importance

First of all, you should visit the award winning C++ FAQ Light, 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 auto_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 aswell. 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.

Link to another (closed) pitfalls thread about C++, so people looking for them will find them:

http://stackoverflow.com/questions/280531/c-pitfalls-closed

Johannes Schaub - litb
a[i] = ++i; //reading a variable twice which is modified leads to undefined behavior ...you can add this also if u wish
yesraaj
+1, many good points. The one about mixing typedef and delete[] was totally new to me! Yet another corner case to remember... :(
j_random_hacker
"Assign 0 to every pointer you delete." <-- Sorry, but wrong. The only solution is to not write the bug in the first place. It's entirely possible someone made a copy of that pointer which will not be affected by you setting it to zero.
Billy ONeal
@BillyONeal, you cannot detect whether you deleted a pointer already if you don't set it to null after you delete it. It's not necessarily a bug to delete twice, if you just set it to null afterwards, thus my proposed solution.
Johannes Schaub - litb
@Johannes Schaub - litb: True, but my point is that's not foolproof. If somebody has a copy of the pointer and tries to delete it you still have double-free problems.
Billy ONeal
@BillyONeal, right. You shouldn't copy the pointer, and then nullify only the original. That's indeed another bug that this won't solve. Not deleting twice on a pointer won't solve it either, because even if you still delete each pointer once - once the copy and once the original, then in the end you have still called delete twice for the same object. This is another pitfall which wasn't meant to be addressed by my entry. I agree with you on that, the real solution for *that* kind of bug is to search and fix that problem.
Johannes Schaub - litb
A: 

This essay/article is very useful, it talks avoid avoiding pitfalls and good practices:

Pointers, references and Values

You can browse the whole site too, which contains programming tips, mainly for C++. I Hope you find it useful.

lurks
A: 

I spent many years doing C++ dev. 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.

Todd Stout
A: 

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.

starblue