4 ms·

C++ pitfalls

54 points by rohshall 14y ago

activepeanut 14y ago
template<typename T> Array<T>::Array(int size) : _size(size), _data(new T(size)) // should have been new T[size] {} That's not the only thing wrong here. private: T* _data; int _size; }; The order of initialization is the order of declaration, NOT the order you use in your constructor. _data depends on _size being initialized first. Therefore _size must be declared above _data. Note: I understand the next example goes over this issue. I just think this example should've been declared properly as to avoid distracting the reader from the main problem, "() vs []".
- aidenn0 14y ago
  Why does _data rely on _size being initialized first? It uses "size" not "_size" in the expression.
  - activepeanut 14y ago
    Good catch. You're correct. I wish I could delete my original comment.
CJefferson 14y ago
This is an interesting list to look through, but things are not as bad as they look. Many of these are warned about on modern compilers, if you use '-Wextra -Wall'. Some of them are even on by default. I think compilers should warn more eagerly. I would put -Wextra -Wall on by default -- it is easier to turn off things you don't want, than to discover things you do not know exist. But I know this is not a popular opinion!
octopus 14y ago
This article is from 1997, you should at least mention this in the title.
- cousin_it 14y ago
  Which of the listed pitfalls are gone in C++11?
  - jeremiep 14y ago
    Probably none, I see programmers falling for these every week at work, it's 2012 now. The C++11 revision didn't change much other than add to the already joyful cursing because now compiler support is even more of an issue than it ever was.
    - lbrandy 14y ago
      There's a few where C++11 has rendered the situation far less common, or the advice not-so-good. For example, the auto_ptr one (advice should be: don't use it). Or the ones that use raw pointer data members and 'delete' in the destructor (for example, the one about the rule of three). This is still good advice, but needed far, far, far more rarely since we can use shared_ptr and unique_ptr to give proper copy/assignment to internal pointers. C++11 has gotten us to a point where using 'new' or 'delete' keywords, even in constructor/destructors are possible code smells.
  - deleted 14y ago
    [deleted]
  - pdw 14y ago
    The second is avoided if you use C++11-style initializers: string a{"Hello"}; string b{}; string c = string{"World"}; Many of the others also disappear if you stick to a "modern" style.
  - pandaman 14y ago
    Not with the C++11 by itself but with better compilers we've got since about 2000 (don't forget that the earliest C++ standard is from 1998): 1. Member initialization order being broken in the constructor initialization is a warning pretty much everywhere (except Microsoft compilers, I think). 2. Having a virtual function and no virtual destructor - a warning even in Microsoft compilers. 3. Trivial infinite recursion (i.e. Manager::print() { print(); } is a warning at least in some compilers. I have seen only a handful of projects where such code existed at all so I cannot say how widespread this warning (For me it popped due to problems with scripts generating code, people don't write this stuff ). This is just from glancing over the article, I am sure if you tried all pitfalls with a modern compiler like clang or even gcc, you'd get a few more warnings. Though I agree with the main point the author is trying to make - C++ is not just Java with pointers so if you are coming from Java background you should be very careful not to assume the conventions you've got used in Java are the same in C++.
16s 14y ago
This is 15 years old.
- wonderzombie 14y ago
  And C++ is approx. twice as old as that. What's your point? :) Seriously, though: I'm no expert, but many people upthread have suggested these are still applicable even in C++11. Compiler warnings help, though you still have to learn what they mean and how to fix them.
lbrandy 14y ago
It's missing my favorite, std::(unordered_)map::operator[] map<int,int> a; cout << a[11] << endl; cout << a.size() << endl; This works fine and has well defined behavior: it will print '0' followed by '1' since the middle line actually inserts a {key=11, value=0}. This is a landmine I think every journeyman c++ programmer steps on a few times.
- blablabla123 14y ago
  I remember being extremely cautious the last time I used std::map... Anyways, g++ -Weffc++ -Wall helps, eventhough not in this case I think ;)
  - npsimons 14y ago
    Start with "g++ --help=warnings | grep "\-W" | awk '{print $1}' | sort | uniq", sort out the ones that don't apply to C++, add in "-pedantic" and "-fdiagnostic-show-option", then (sparingly!) disable spurious warnings via the "GCC diagnostic ignored" pragma, or just outright removal of the warning option (the first candidate that comes to mind is -Waggregate-return). Do be aware that if you are using libraries which trigger warnings, you can turn off warning checking for them with "-isystem" (but make sure to also take "-Wsystem-header" out of your list of warnings). This won't prevent every gun pointed at foot incident, but it's a good start.
  - zem 14y ago
    wow, did not know about -Weffc++. quite impressive that they added that flag. here's a blog post if anyone else wants to read about it: http://cpptruths.blogspot.com/2006/08/g-compiler-option-weffc.html http://cpptruths.blogspot.com/2006/08/g-compiler-option-weff...
- angersock 14y ago
  The amazing shittiness of std::map and friends drove us to write our own, with blackjack and hookers. I really wish there was an API "Hall of Shame" with attached discussion minutes showing the exact points where a group of otherwise sane people decided to kludge these things in.
430gj9j 14y ago
All languages have pitfalls. Including the author's beloved Java. For example: String x = "foo"; String y = "foo"; // Appears to work but compares references System.out.println(x[0] == x[1]); There's no substitute for knowing your tools well.
- deleted 14y ago
  [deleted]
- diek 14y ago
  I assume you meant to say: System.out.println(x == y);
  - 430gj9j 14y ago
    Yes. In a previous version I was trying to bypass string caching to make x==y return false.
- wonderzombie 14y ago
  The difference is that languages can have more pitfalls, more severe pitfalls, pitfalls which are more difficult to detect, and so on. I don't love Java, not by any stretch, but it's difficult to argue that Java and C++ are anywhere near equivalent in this dimension. Effective C++ is littered with examples of gotchas which are highly peculiar to C++ and the C++ compiler. It's an incredibly complex language. Whether C++ worth these trade-offs for a particular team, project, or company is orthogonal.
givan 14y ago
C++ provides high level stuff at the expense of making the programmer part of the compiler that must give it hints about memory, types or cope with multiple page error outputs because of one missing semicolon. C++ exists because real high level languages does not have very efficient compilers yet. And where high performance is really needed and programmers time or effort is not a problem C++ is preferred. I expect that with the increasing processing power compilers will get some AI incorporated and get smarter in the future and high level languages will output binaries that are at least as fast as those produced by C++ or C compilers.
- npsimons 14y ago
  C++ exists because processing power and RAM used to be at a premium; these days, hardware is so cheap, you can get away with writing inefficient code in any language. However, two things are of note here: C++ has had a lot of higher level features bolted on after the fact, including compiler warnings and tools to help reduce burden on the programmer, and higher level languages have had a lot of performance "bolted on" (plus hardware has gotten so much faster and cheaper) so that if you code properly (in any language), performance differences are near negligible. C++ mostly hangs on because of backwards compatibility and a large user base; I suspect that as time goes on, we'll see it fall more and more by the wayside (and I say this as someone whose bread and butter is C++), especially with the fact that programmer time is a big cost driver.
  - comicjk 14y ago
    If you're writing games or scientific programs, you still need C++, if not Fortran or assembly.
gregsqueeb 14y ago
Do you go to SJSU? Hit me up! gregdmathews@gmail.com

54 points by rohshall 14y ago

template<typename T> Array<T>::Array(int size) : _size(size), _data(new T(size)) // should have been new T[size] {} That's not the only thing wrong here. private: T* _data; int _size; }; The order of initialization is the order of declaration, NOT the order you use in your constructor. _data depends on _size being initialized first. Therefore _size must be declared above _data. Note: I understand the next example goes over this issue. I just think this example should've been declared properly as to avoid distracting the reader from the main problem, "() vs []".

aidenn0 14y ago
Why does _data rely on _size being initialized first? It uses "size" not "_size" in the expression.
- activepeanut 14y ago
  Good catch. You're correct. I wish I could delete my original comment.

CJefferson 14y ago

This is an interesting list to look through, but things are not as bad as they look. Many of these are warned about on modern compilers, if you use '-Wextra -Wall'. Some of them are even on by default. I think compilers should warn more eagerly. I would put -Wextra -Wall on by default -- it is easier to turn off things you don't want, than to discover things you do not know exist. But I know this is not a popular opinion!

octopus 14y ago

This article is from 1997, you should at least mention this in the title.

cousin_it 14y ago
Which of the listed pitfalls are gone in C++11?
- jeremiep 14y ago
  Probably none, I see programmers falling for these every week at work, it's 2012 now. The C++11 revision didn't change much other than add to the already joyful cursing because now compiler support is even more of an issue than it ever was.
  - lbrandy 14y ago
    There's a few where C++11 has rendered the situation far less common, or the advice not-so-good. For example, the auto_ptr one (advice should be: don't use it). Or the ones that use raw pointer data members and 'delete' in the destructor (for example, the one about the rule of three). This is still good advice, but needed far, far, far more rarely since we can use shared_ptr and unique_ptr to give proper copy/assignment to internal pointers. C++11 has gotten us to a point where using 'new' or 'delete' keywords, even in constructor/destructors are possible code smells.
- deleted 14y ago
  [deleted]
- pdw 14y ago
  The second is avoided if you use C++11-style initializers: string a{"Hello"}; string b{}; string c = string{"World"}; Many of the others also disappear if you stick to a "modern" style.
- pandaman 14y ago
  Not with the C++11 by itself but with better compilers we've got since about 2000 (don't forget that the earliest C++ standard is from 1998): 1. Member initialization order being broken in the constructor initialization is a warning pretty much everywhere (except Microsoft compilers, I think). 2. Having a virtual function and no virtual destructor - a warning even in Microsoft compilers. 3. Trivial infinite recursion (i.e. Manager::print() { print(); } is a warning at least in some compilers. I have seen only a handful of projects where such code existed at all so I cannot say how widespread this warning (For me it popped due to problems with scripts generating code, people don't write this stuff ). This is just from glancing over the article, I am sure if you tried all pitfalls with a modern compiler like clang or even gcc, you'd get a few more warnings. Though I agree with the main point the author is trying to make - C++ is not just Java with pointers so if you are coming from Java background you should be very careful not to assume the conventions you've got used in Java are the same in C++.

16s 14y ago

This is 15 years old.

wonderzombie 14y ago
And C++ is approx. twice as old as that. What's your point? :) Seriously, though: I'm no expert, but many people upthread have suggested these are still applicable even in C++11. Compiler warnings help, though you still have to learn what they mean and how to fix them.

lbrandy 14y ago

It's missing my favorite, std::(unordered_)map::operator[] map<int,int> a; cout << a[11] << endl; cout << a.size() << endl; This works fine and has well defined behavior: it will print '0' followed by '1' since the middle line actually inserts a {key=11, value=0}. This is a landmine I think every journeyman c++ programmer steps on a few times.

blablabla123 14y ago
I remember being extremely cautious the last time I used std::map... Anyways, g++ -Weffc++ -Wall helps, eventhough not in this case I think ;)
- npsimons 14y ago
  Start with "g++ --help=warnings | grep "\-W" | awk '{print $1}' | sort | uniq", sort out the ones that don't apply to C++, add in "-pedantic" and "-fdiagnostic-show-option", then (sparingly!) disable spurious warnings via the "GCC diagnostic ignored" pragma, or just outright removal of the warning option (the first candidate that comes to mind is -Waggregate-return). Do be aware that if you are using libraries which trigger warnings, you can turn off warning checking for them with "-isystem" (but make sure to also take "-Wsystem-header" out of your list of warnings). This won't prevent every gun pointed at foot incident, but it's a good start.
- zem 14y ago
  wow, did not know about -Weffc++. quite impressive that they added that flag. here's a blog post if anyone else wants to read about it: http://cpptruths.blogspot.com/2006/08/g-compiler-option-weffc.html http://cpptruths.blogspot.com/2006/08/g-compiler-option-weff...
angersock 14y ago
The amazing shittiness of std::map and friends drove us to write our own, with blackjack and hookers. I really wish there was an API "Hall of Shame" with attached discussion minutes showing the exact points where a group of otherwise sane people decided to kludge these things in.

430gj9j 14y ago

All languages have pitfalls. Including the author's beloved Java. For example: String x = "foo"; String y = "foo"; // Appears to work but compares references System.out.println(x[0] == x[1]); There's no substitute for knowing your tools well.

deleted 14y ago
[deleted]
diek 14y ago
I assume you meant to say: System.out.println(x == y);
- 430gj9j 14y ago
  Yes. In a previous version I was trying to bypass string caching to make x==y return false.
wonderzombie 14y ago
The difference is that languages can have more pitfalls, more severe pitfalls, pitfalls which are more difficult to detect, and so on. I don't love Java, not by any stretch, but it's difficult to argue that Java and C++ are anywhere near equivalent in this dimension. Effective C++ is littered with examples of gotchas which are highly peculiar to C++ and the C++ compiler. It's an incredibly complex language. Whether C++ worth these trade-offs for a particular team, project, or company is orthogonal.

givan 14y ago

C++ provides high level stuff at the expense of making the programmer part of the compiler that must give it hints about memory, types or cope with multiple page error outputs because of one missing semicolon. C++ exists because real high level languages does not have very efficient compilers yet. And where high performance is really needed and programmers time or effort is not a problem C++ is preferred. I expect that with the increasing processing power compilers will get some AI incorporated and get smarter in the future and high level languages will output binaries that are at least as fast as those produced by C++ or C compilers.

npsimons 14y ago
C++ exists because processing power and RAM used to be at a premium; these days, hardware is so cheap, you can get away with writing inefficient code in any language. However, two things are of note here: C++ has had a lot of higher level features bolted on after the fact, including compiler warnings and tools to help reduce burden on the programmer, and higher level languages have had a lot of performance "bolted on" (plus hardware has gotten so much faster and cheaper) so that if you code properly (in any language), performance differences are near negligible. C++ mostly hangs on because of backwards compatibility and a large user base; I suspect that as time goes on, we'll see it fall more and more by the wayside (and I say this as someone whose bread and butter is C++), especially with the fact that programmer time is a big cost driver.
- comicjk 14y ago
  If you're writing games or scientific programs, you still need C++, if not Fortran or assembly.

gregsqueeb 14y ago

Do you go to SJSU? Hit me up! gregdmathews@gmail.com