C++ pitfalls | Hacker News Reader

C++ pitfalls

horstmann.com

54 pointsrohshall31 comments

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.

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.

I agree this particular example can be confusing the first time you hit it, especially coming from other languages.

It's better than the alternatives though, I would be interested to hear how you handle this in your version. Leaving the behaviour undefined for non-existent keys is likely to cause far worse bugs, throwing an exception would be inconsistent with the rest of the stl.

The could have left it out altogether, but would mean losing some nice properties - operator[] returning a reference makes it possible to assign into the map directly ( a[3] = 5; ). Also since the value is default initialized, you can write something like a counter easily, much like a python defaultdict:

  for (auto id in ids) {
    a[id] += 10;
  }

You can always stick to .find and .insert if you prefer the more explicit behaviour.

In the version we ended up writing, 'operator []' is equivalent to a call to 'Get', which also returns a reference to the mapped value. In the event that key is not mapped, it asserts. If you handle the assert or have disabled runtime asserts, it returns a reference to a static value instance, so you can handle this kind of error yourself or even use the static value as a 'default' (though we never use it that way).

Though not consistent with how STL works, it is consistent with how our containers work and how we use maps. YMMV.

Also, since we wrote it, we're free to change the behavior if a better way manifests itself. So, if you have any suggestions, let fly.

You could return a proxy object that offers operator= et al, and an implicit conversion operator to T& or T const& that throws an exception if that key is not in the map.

(I don't know if that would be better, since it's even more magic and it still isn't perfect.)

I like the style of your comment; it made me laugh aloud in an airport and brightened an otherwise shitty day. I especially like your characterization of C++ Std API designers as "otherwise sane people," which isn't exactly how I imagine them.

Now, off to pour Courvoisier on those funny bumps in my groin...

I remember being extremely cautious the last time I used std::map...

Anyways, g++ -Weffc++ -Wall helps, eventhough not in this case I think ;)

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.

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

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!

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.

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.

If you're writing games or scientific programs, you still need C++, if not Fortran or assembly.

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

Which of the listed pitfalls are gone in C++11?

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

I never understood the need for blanket warnings for member initialization order on gcc. 99% of the time the order doesn't matter (who cares if I assign zero to one int or another first). And when it does, it is better to move initialization to the body of constructor anyway. Basically I have to do annoying reordering every time I do some changes on msvc and then compile on gcc just to remove the warnings.

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.

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.

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.

With explicit conversion operators you can fix this: "if (-0.5 <= x <= 0.5) return 0;" for x that are not built-in types. You would need to create your own Bool type, something like defined below, and overload <= operators for x type, returning Bool. This way you will get compile time error.

class Bool { bool value; public: Bool(bool v) : value(v) {} explicit operator bool () { return value; } };

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.

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.

I assume you meant to say:

    System.out.println(x == y);

Yes. In a previous version I was trying to bypass string caching to make x==y return false.

  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 []".

Why does _data rely on _size being initialized first? It uses "size" not "_size" in the expression.

Good catch. You're correct. I wish I could delete my original comment.

This is 15 years old.

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.

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

Reply on news.ycombinator.com