const and mutable have been in C++ for many years. How well do you know what they mean today?
1. What is a “shared variable”?
2. What do const and mutable mean on shared variables?
3. How are const and mutable different in C++98 and C++11?
Virtual functions are a pretty basic feature, but they occasionally harbor subtleties that trap the unwary. If you can answer questions like this one, then you know virtual functions cold, and you’re less likely to waste a lot of time debugging problems like the ones illustrated below.
1. What do the override and final keywords do? Why are they useful?
2. In your travels through the dusty corners of your company’s code archives, you come across the following program fragment written by an unknown programmer. The programmer seems to have been experimenting to see how some C++ features worked.
(a) What could be improved in the code’s correctness or style?
(b) What did the programmer probably expect the program to print, but what is the actual result?
class base {
public:
virtual void f( int );
virtual void f( double );
virtual void g( int i = 10 );
};
void base::f( int ) {
cout << "base::f(int)" << endl;
}
void base::f( double ) {
cout << "base::f(double)" << endl;
}
void base::g( int i ) {
cout << i << endl;
}
class derived: public base {
public:
void f( complex<double> );
void g( int i = 20 );
};
void derived::f( complex<double> ) {
cout << "derived::f(complex)" << endl;
}
void derived::g( int i ) {
cout << "derived::g() " << i << endl;
}
int main() {
base b;
derived d;
base* pb = new derived;
b.f(1.0);
d.f(1.0);
pb->f(1.0);
b.g();
d.g();
pb->g();
delete pb;
}
These keywords give explicit control over virtual function overriding. Writing override declares the intent to override a base class virtual function. Writing final makes a virtual function no longer overrideable in further-derived classes, or a class no longer permitted to have further-derived classes.
They are useful because they let the programmer explicitly declare intent in a way the language can enforce at compile time. If you write override but there is no matching base class function, or you write final and a further-derived class tries to implicitly or explicitly override the function anyway, you get a compile-time error.
Of the two, by far the more commonly useful is override; uses for final are rarer.
First, let’s consider some style issues, and one real error:
Avoid using owning raw pointers and explicit new and delete except in rare cases like when you’re writing the internal implementation details of a low-level data structure.
{
base* pb = new derived;
...
delete pb;
}
Instead of new and base*, use make_unique and unique_ptr<base>.
{
auto pb = unique_ptr<base>{ make_unique<derived>() };
...
} // automatic delete here
Guideline: Don’t use explicit new, delete, and owning * pointers, except in rare cases encapsulated inside the implementation of a low-level data structure.
However, that delete brings us to another issue unrelated to how we allocate and manage the lifetime of the object, namely:
class base {
public:
virtual void f( int );
virtual void f( double );
virtual void g( int i = 10 );
};
This looks innocuous, but the writer of base forgot to make the destructor either virtual or protected. As it is, deleting via a pointer-to-base without a virtual destructor is evil, pure and simple, and corruption is the best thing you can hope for because the wrong destructor will get called, derived class members won’t be destroyed, and operator delete will be invoked with the wrong object size.
Guideline: Make base class destructors public and virtual, or protected and nonvirtual.
Exactly one of the following can be true for a polymorphic type:
For the next few points, it’s important to differentiate three terms:
void derived::f( complex<double> )
derived does not overload the base::f functions, it hides them. This distinction is very important, because it means that base::f(int) and base::f(double) are not visible in the scope of derived.
If the author of derived intended to hide the base functions named f, then this is all right. Usually, however, the hiding is inadvertent and surprising, and the correct way to bring the names into the scope of derived is to write the using-declaration using base::f; inside derived.
Guideline: When providing a non-overridden function with the same name as an inherited function, be sure to bring the inherited functions into scope with a using-declaration if you don’t want to hide them.
void g( int i = 20 ) /* override */
This function overrides the base function, so it should say override explicitly. This documents the intent, and lets the compiler tell you if you’re trying to override something that’s not virtual or you got the signature wrong by mistake.
Guideline: Always write override when you intend to override a virtual function.
void g( int i = 20 )
Changing the default argument is decidedly user-unfriendly. Unless you’re really out to confuse people, don’t change the default arguments of the inherited functions you override. Yes, this is legal C++, and yes, the result is well-defined; and no, don’t do it. Further below, we’ll see just how confusing this can be.
Guideline: Never change the default arguments of overridden inherited functions.
We could go one step further:
Guideline: Avoid default arguments on virtual functions in general.
Finally, public virtual functions are great when a class is acting as a pure abstract base class (ABC) that only specifies the virtual interface without implementations, like a C# or Java interface does.
Guideline: Prefer to have a class contain only public virtual functions, or no public virtual functions (other than the destructor which is special).
A pure abstract base class should have only public virtual functions. …
But when a class is both providing virtual functions and their implementations, consider the Non-Virtual Interface pattern (NVI) that makes the public interface and the virtual interface separate and distinct.
… For any other base class, prefer making public member functions non-virtual, and virtual member functions non-public; the former should have any default arguments and can be implemented in terms of the latter.
This cleanly separates the public interface from the derivation interface, lets each follow its natural form best suited for its distinct audience, and avoids having one function exist in tension from doing double duty with two responsibilities. Among other benefits, using NVI will often clarify your class’s design in important ways, including for example that the default arguments which matter to the caller therefore naturally belong on the public interface, not on the virtual interface. Following this pattern means that several classes of potential problems, including this one of virtuals with default arguments, just naturally don’t arise.
The C++ standard library follows NVI nearly universally, and other modern OO languages and environments have rediscovered this principle for their own library design guidelines, such as in the .NET Framework Design Guidelines.
Now that we have those issues out of the way, let’s look at the mainline and see whether it does that the programmer intended:
int main() {
base b;
derived d;
base* pb = new derived;
b.f(1.0);
No problem. This first call invokes base::f( double ), as expected.
d.f(1.0);
This calls derived::f( complex<double> ). Why? Well, remember that derived doesn’t declare using base::f; to bring the base functions named f into scope, and so clearly base::f( int ) and base::f( double ) can’t be called. They are not present in the same scope as derived::f( complex<double> ) so as to participate in overloading.
The programmer may have expected this to call base::f( double ), but in this case there won’t even be a compile error because fortunately(?) complex<double> provides an implicit conversion from double, and so the compiler interprets this call to mean derived::f( complex<double>(1.0) ).
pb->f(1.0);
Interestingly, even though the base* pb is pointing to a derived object, this calls base::f( double ) because overload resolution is done on the static type (here base), not the dynamic type (here derived). You have a base pointer, you get the base interface.
For the same reason, the call pb->f(complex<double>(1.0)); would not compile, because there is no satisfactory function in the base interface.
b.g();
This prints 10, because it simply invokes base::g( int ) whose parameter defaults to the value 10. No sweat.
d.g();
This prints derived::g() 20, because it simply invokes derived::g( int ) whose parameter defaults to the value 20. Also no sweat.
pb->g();
This prints derived::g() 10.
“Wait a minute!” you might protest. “What’s going on here?” This result may temporarily lock your mental brakes and bring you to a screeching halt until you realize that what the compiler has done is quite proper. (Although, of course, the programmer of derived ought to be taken out into the back parking lot and yelled at.) The thing to remember is that, like overloads, default parameters are taken from the static type (here base) of the object, hence the default value of 10 is taken. However, the function happens to be virtual, and so the function actually called is based on the dynamic type (here derived) of the object. Again, this can be avoided by avoiding default arguments on virtual functions, such as by following NVI and avoiding public virtual functions entirely.
delete pb;
}
Finally, as noted, this shouldn’t be needed because you should be using unique_ptrs which do the cleanup for you, and base should have a virtual destructor so that destruction via any pointer to base is correct.
Thanks in particular to the following for their feedback to improve this article: litb1, KrzaQ, mttpd.
Virtual functions are a pretty basic feature, but they occasionally harbor subtleties that trap the unwary. If you can answer questions like this one, then you know virtual functions cold, and you’re less likely to waste a lot of time debugging problems like the ones illustrated below.
1. What do the override and final keywords do? Why are they useful?
2. In your travels through the dusty corners of your company’s code archives, you come across the following program fragment written by an unknown programmer. The programmer seems to have been experimenting to see how some C++ features worked.
(a) What could be improved in the code’s correctness or style?
(b) What did the programmer probably expect the program to print, but what is the actual result?
class base {
public:
virtual void f( int );
virtual void f( double );
virtual void g( int i = 10 );
};
void base::f( int ) {
cout << "base::f(int)" << endl;
}
void base::f( double ) {
cout << "base::f(double)" << endl;
}
void base::g( int i ) {
cout << i << endl;
}
class derived: public base {
public:
void f( complex<double> );
void g( int i = 20 );
};
void derived::f( complex<double> ) {
cout << "derived::f(complex)" << endl;
}
void derived::g( int i ) {
cout << "derived::g() " << i << endl;
}
int main() {
base b;
derived d;
base* pb = new derived;
b.f(1.0);
d.f(1.0);
pb->f(1.0);
b.g();
d.g();
pb->g();
delete pb;
}
How good are you at the details of writing classes? This item focuses not only on blatant errors, but even more so on professional style. Understanding these principles will help you to design classes that are easier to use and easier to maintain.
1. What makes interfaces “easy to use correctly, hard to use incorrectly”? Explain.
2. You are doing a code review. A programmer has written the following class, which shows some poor style and has some real errors. How many can you find, and how would you fix them?
class complex {
public:
complex( double r, double i = 0 )
: real(r), imag(i)
{ }
void operator+ ( complex other ) {
real = real + other.real;
imag = imag + other.imag;
}
void operator<<( ostream os ) {
os << "(" << real << "," << imag << ")";
}
complex operator++() {
++real;
return *this;
}
complex operator++( int ) {
auto temp = *this;
++real;
return temp;
}
// ... more functions that complement the above ...
private:
double real, imag;
};
Note: This is not intended to be a complete class. For example, if you provide operator++ you would normally also provide operator–. Rather, this is an instructive example to focus on the mechanics of writing correctly the kinds of functions this class is trying to support.
We want to enable a “pit of success” where users of our type just naturally fall into good practices—they just naturally write code that is valid, correct, and efficient.
On the other hand, we want to make it hard for our users to get into trouble—we want code that would be incorrect or inefficient to be invalid (a compile time error if possible) or at least inconvenient and hard to write silently so that we can protect the user from unwelcome surprises.
Scott Meyers popularized this guidance. See his concise writeup for further examples.
This class has a lot of problems—even more than I will show explicitly here. The point of this puzzle was primarily to highlight class mechanics (issues like “what is the canonical form of operator<<?” and “should operator+ be a member?”) rather than point out where the interface is just plain poorly designed. However, I will start off with perhaps the two most useful observation first:
First, this is a code review but the developer doesn’t seem to have tried to even unit-test his code, else he would have found some glaring problems.
Second, why write a complex class when one already exists in the standard library? And, what’s more, when the standard one isn’t plagued with any of the following problems and has been crafted based on years of practice by the best people in our industry? Humble thyself and reuse.
Guideline: Reuse code—especially standard library code—instead of handcrafting your own. It’s faster, easier, and safer.
Perhaps the best way to fix the problems in the complex code is to avoid using the class at all, and use the std::complex template instead.
Having said that, it’s an instructive example, so let’s go through the class as written and fix the problems as we go. First, the constructor:
complex( double r, double i = 0 )
: real(r), imag(i)
{ }
Once we supply a user-written constructor, we suppress the implicit generation of the default constructor. Beyond “easy to use correctly,” not having a default constructor makes the class annoying to use at all. In this case, we could either default both parameters, or provide a complex() = default; and declare the data members with initializers such as double real = 0, imag = 0; , or just delegate with complex() : complex(0) { } . Just defaulting the parameter is the simplest here.
Also, as explained in GotW #1, prefer to use { } consistently for initialization rather than ( ) just as a good modern habit. The two mean exactly the same thing in this case, but { } lets us be more consistent, and could catch a few errors during maintenance, such as typos that would invoke double-to-float narrowing conversions.
void operator+ ( complex other ) {
real = real + other.real;
imag = imag + other.imag;
}
Although we’re about make other changes to this function in a moment, as written this parameter should be passed by const& because all we do is read from it.
Guideline: Prefer passing a read-only parameter by const& if you are only going to read from it (not make a copy of it).
Instead of returning void, operator+ should return a complex containing the sum and not modify this object’s value. Users who write val1 + val2 and see val1 changed are unlikely to be impressed by these gratuitously weird semantics. As Scott Meyers is wont to say, when writing a value type, “do as the ints do” and follow the conventions of the built-in types.
Really, this operator+ is trying to be operator+=. It should be split into an actual operator+ and operator+=, with the former calling the latter.
Guideline: If you supply a standalone version of an operator (e.g., operator+), always supply an assignment version of the same operator (e.g., operator+=) and prefer implementing the former in terms of the latter. Also, always preserve the natural relationship between op and op= (where op stands for any operator).
Having += is good, because users should prefer using it. Even in the above code, real = real + other.real; should be real += other.real; and similarly for the second line.
Guideline: Prefer writing a op= b instead of a = a op b (where op stands for any operator). It’s clearer, and it’s often more efficient.
The reason why operator+= is more efficient is that it operates on the left-hand object directly and returns only a reference, not a temporary object. On the other hand, operator+ must return a temporary object. To see why, consider the following canonical forms for how operator+= and operator+ should normally be implemented for some type T.
T& T::operator+=( const T& other ) {
//...
return *this;
}
T operator+( T a, const T& b ) {
a += b;
return a;
}
Did you notice that one parameter is passed by value, and one by reference? That’s because if you’re going to copy from a parameter anyway, it’s often better to pass it by value, which will naturally enable a move operation if the caller passes a temporary object such as in expressions like (val1 * val2) + val3. This is a good habit to follow even in cases like complex where a move is the same cost as a copy, since it doesn’t cost any efficiency when move and copy are the same, and arguably makes for cleaner code than passing by reference and adding an extra named local object. We’ll see more on parameter passing in a future GotW.
Guideline: Prefer passing a read-only parameter by value if you’re going to make a copy of the parameter anyway, because it enables move from rvalue arguments.
Implementing + in terms of += both makes the code simpler and guarantees consistent semantics as the two functions are less likely to diverge during maintenance.
If operator+ is made a member function, as it is here, then it won’t work as naturally as your users may expect when you do decide to allow implicit conversions from other types. Here, an implicit conversion from double to complex makes sense, but with the original class users have an asymmetry: Specifically, when adding complex objects to numeric values, you can write a = b + 1.0 but not a = 1.0 + b because a member operator+ requires a complex (and not a double) as its left-hand argument.
Finally, the other reason to prefer non-members is because they provide better encapsulation, as pointed out by Scott Meyers.
Guideline: Prefer these guidelines for making an operator a member vs. nonmember function: unary operators are members; = () [] and -> must be members; the assignment operators (+= –= /= *= etc.) must be members; all other binary operators are nonmembers.
The author of this code didn’t really mean to enable the syntax my_complex << cout, did they?
void operator<<( ostream os ) {
os << "(" << real << "," << imag << ")";
}
The same reasons already given to show why operator+ should be a nonmember apply also to operator<<, only more so because a member the first parameter has to be a stream, not a complex. Further, the parameters should be references: (ostream&, const complex &).
Note also that the nonmember operator<< should normally be implemented in terms of a(n often virtual) const member function that does the work, usually named something like print.
Further, operator<< should have a return type of ostream& and should return a reference to the stream in order to permit chaining. That way, users can use your operator<< naturally in code like cout << a << b;.
Guideline: Always return stream references from operator<< and operator>>.
complex operator++() {
++real;
return *this;
}
Ignoring for the sake of argument whether preincrement is meaningful for complex numbers, if the function exists it should return a reference. This lets client code operate more intuitively and avoids needless inefficiency.
Guideline: When you return *this, the return type should usually be a reference.
complex operator++( int ) {
auto temp = *this;
++real;
return temp;
}
Instead of repeating the work, prefer to call ++*this. See GotW #2 for the full canonical form for postincrement.
Guideline: For consistency, always implement postincrement in terms of preincrement, otherwise your users will get surprising (and often unpleasant) results.
That’s it. There are other modern C++ features we could apply here, but they would be arguably gratuitous and not appropriate for general recommendations. For example, this is a value type not designed to be inherited from, so we could prevent inheritance by making the class final, but that would be protecting against Machiavelli, not Murphy, and there’s no need for a general guideline that tells everyone they should now write final on every value type; that would just be tedious and unnecessary.
Here’s a corrected version of the class, ignoring design and style issues not explicitly noted above:
class complex {
public:
complex( double r = 0, double i = 0 )
: real{r}, imag{i}
{ }
complex& operator+=( const complex& other ) {
real += other.real;
imag += other.imag;
return *this;
}
complex& operator++() {
++real;
return *this;
}
complex operator++( int ) {
auto temp = *this;
++*this;
return temp;
}
ostream& print( ostream& os ) const {
return os << "(" << real << "," << imag << ")";
}
private:
double real, imag;
};
complex operator+( complex lhs, const complex& rhs ) {
lhs += rhs;
return lhs;
}
ostream& operator<<( ostream& os, const complex& c ) {
return c.print(os);
}
Thanks in particular to the following for their feedback to improve this article: Mikhail Belyaev, jlehrer, Olaf van der Spek, Marshall, litb1, hm, Dave Harris, nosenseetal.
How good are you at the details of writing classes? This item focuses not only on blatant errors, but even more so on professional style. Understanding these principles will help you to design classes that are easier to use and easier to maintain.
1. What makes interfaces “easy to use correctly, hard to use incorrectly”? Explain.
2. You are doing a code review. A programmer has written the following class, which shows some poor style and has some real errors. How many can you find, and how would you fix them?
class complex {
public:
complex( double r, double i = 0 )
: real(r), imag(i)
{ }
void operator+ ( complex other ) {
real = real + other.real;
imag = imag + other.imag;
}
void operator<<( ostream os ) {
os << "(" << real << "," << imag << ")";
}
complex operator++() {
++real;
return *this;
}
complex operator++( int ) {
auto temp = *this;
++real;
return temp;
}
// ... more functions that complement the above ...
private:
double real, imag;
};
Note: This is not intended to be a complete class. For example, if you provide operator++ you would normally also provide operator–. Rather, this is an instructive example to focus on the mechanics of writing correctly the kinds of functions this class is trying to support.
Effective reuse is an important part of good software engineering. To demonstrate how much better off you can be by using standard library algorithms instead of handcrafting your own, let’s reconsider the previous question to demonstrate how many of the problems could have been avoided by simply reusing what’s already available in the standard library.
1. What is the most widely used C++ library?
2. How many of the pitfalls in GotW #2 could have been avoided in the first place, if only the programmer had replaced the explicit iterator-based for loop with:
(a) a range-based for loop?
(b) a standard library algorithm call?
Demonstrate. (Note: As with GotW #2, don’t change the semantics of the function, even though they could be improved.)
To recap, here is the mostly-fixed function:
string find_addr( const list<employee>& emps, const string& name ) {
for( auto i = begin(emps); i != end(emps); ++i ) {
if( i->name() == name ) {
return i->addr;
}
}
return "";
}
The C++ standard library, with its implementations on every platform.
Astute readers of GotW #2 will have been champing at the bit to say: “Why aren’t you using a range-based for loop?” Indeed, why not? That would solve several of the temporaries, never mind be easier to write.
Compare the original unimproved explicit iterator loop:
for( auto i = begin(emps); i != end(emps); i++ ) {
if( *i == name ) {
return i->addr;
}
}
with the range-based for loop (bonus points if you remembered to write the const auto&):
for( const auto& e : emps ) {
if( e == name ) {
return e.addr;
}
}
The expressions e == name and return e.addr; are unchanged in terms of their possible or actual temporaries. But the questions in the naked loop code about whether or not the = causes a temporary (recall: it doesn’t), whether or not end() recalculation matters and should be hoisted (recall: probably not, but maybe), and whether or not i++ should be rewritten ++i (recall: it should) all simply don’t arise in the range-for code. Such is the power of clear code, and using a higher level of abstraction.
A key advantage is that using the range-based for loop has increased our level of abstraction, the information density in our code. Consider: What can you say about the following two pieces of code without reading what comes next?
for( auto i = begin(emps); i != end(emps); i++ ) { // A
for( const auto& e : emps ) { // B
At first it might seem that lines A and B convey the same information, but they don’t. When you see A, all you know is that there’s a loop of some sort that uses an iterator over emps. Granted, we’re so used to A that our eye’s peripheral vision tends to “autocomplete” it in our heads into “a loop that visits the elements of emps in order” and our autocomplete is often correct—except when it isn’t: was that a ++, or a s+= 2 in a strided loop? is the index modified inside the body? Our peripheral vision might be wrong.
On the other hand, B conveys more information to the reader. When you see B, you know for certain without inspecting the body of the loop that it is a loop that visits the element of emps in order. What’s more, you’ve simplified the loop control because there’s no need for an iterator indirection. Both of these are raising the level of abstraction of our code, and that’s a good thing.
Note that, as discussed in GotW #2, the naked for loop didn’t naturally allow consolidating to a single return statement without resorting to making the code more complex by adding an additional variable and performing extra computation (a default construction followed by an assignment, instead of just a construction). That’s still true of the range-based for loop form, because it still has the two return statements in different scopes.
With no other changes, simply using the standard find algorithm could do everything the range-based for loop did to avoid needless temporaries (and questions about them):
// Better (focusing on internals)
//
string find_addr( /*...*/ ) {
const auto i = find( begin(emps), end(emps), name ); // TFTFY
return i != end(emps) ? i->addr : "";
}
This naturally eliminates the same temporaries as the range-for version, and it further increases our level of abstraction. As with the range-based for loop, we can see at a glance and for certain that the loop will visit the elements of emps in order, but on top of that we also know we’re trying to find something and will get back an iterator to the first matching element if one exists. We do still have an iterator indirection, but only a single-use iterator object and no iterator arithmetic as in the original naked iterator for loop.
Further, we have eliminated a loop nested scope entirely and flattened out the function to a single scope which can simplify this calling function in ways even the range-for couldn’t. To demonstrate still more just how fundamental this point is, note that what else the flattening out of the body buys us: Now, because the return statements are in the same scope (possible only because we eliminated the loop scope), we have the option of naturally combining them. You could still write if( i != end(emps) ) return i->addr; else return “”; here, on one or two or four lines, but there’s no need to. To be clear, the point here is not that reducing return statements should be a goal in itself—it shouldn’t be, and “single exit” thinking has always been flawed as we already saw in GotW #2. Rather, the point is that using an algorithm often simplifies our code more than an explicit loop, even a range-for loop, can do—not only directly by removing extra indirections and extra variables and a loop nested scope, but often also by permitting additional simplifications in nearby code.
The above code might still cause a temporary when comparing an employee with a string, and we can eliminate even that temporary if we go one small step further and use find_if with a custom comparison that compares e.name() == name to avoid a possible conversion, assuming something like a suitable employee::name() is available as we did in GotW #2. Combining this with the other fixes to pass parameters by reference, we get:
// Better still (complete)
//
string find_addr( const list<employee>& emps, const string& name ) {
const auto i = find_if( begin(emps), end(emps),
[&](const auto& e) { return e.name() == name; } );
return i != end(emps) ? i->addr : "";
}
Prefer algorithm calls over explicit loops, when you have or can write a suitable algorithm that does what you want. They raise the level of abstraction and the clarity of our code. Scott Meyers’ advice in Effective STL is still true, and more applicable than even now that lambdas make algorithms much more usable than before:
Guideline: Prefer algorithm calls to explicit loops. Algorithm calls are often clearer and reduce complexity. If no suitable algorithm exists, why not write it? You’ll use it again.
Prefer reusing existing library code to handcrafting your own. The more widely used the library, the more likely it is to come well-designed, pre-debugged, and pre-optimized for many common requirements. And what library is more widely used than the standard library? In your C++ program, your standard library implementation is the most widely used library code you’re likely to use. This helps you both in the library’s design and its implementation: It’s full of code that’s intended to be used and reused, and to that end a lot of thought and care has gone into the design of its features, including its standard algorithms like find and sort. Implementers have also spent hours sweating over efficiency details, and usability details, and all sorts of other considerations so that you don’t have to—including performing optimizations you should almost never resort to in application-level code, such as using nonportable OS- and CPU-target specific optimizations.
So, always prefer to reuse code, especially algorithms and especially the standard library, and escape the trap of “I’ll-write-my-own-just-’cause-I-can.”
Guideline: Reuse code—especially standard library code—instead of handcrafting your own. It’s faster, easier, and safer.
Thanks in particular to the following for their feedback to improve this article: Olaf ven der Spek, Sam Kramer.
Effective reuse is an important part of good software engineering. To demonstrate how much better off you can be by using standard library algorithms instead of handcrafting your own, let’s reconsider the previous question to demonstrate how many of the problems could have been avoided by simply reusing what’s already available in the standard library.
1. What is the most widely used C++ library?
2. How many of the pitfalls in GotW #2 could have been avoided in the first place, if only the programmer had replaced the explicit iterator-based for loop with:
(a) a range-based for loop?
(b) a standard library algorithm call?
Demonstrate. (Note: As with GotW #2, don’t change the semantics of the function, even though they could be improved.)
To recap, here is the mostly-fixed function:
string find_addr( const list<employee>& emps, const string& name ) {
for( auto i = begin(emps); i != end(emps); ++i ) {
if( i->name() == name ) {
return i->addr;
}
}
return "";
}
Unnecessary and/or temporary objects are frequent culprits that can throw all your hard work—and your program’s performance—right out the window. How can you spot them and avoid them?
1. What is a temporary object?
2. You are doing a code review. A programmer has written the following function, which uses unnecessary temporary or extra objects in at least three places. How many can you identify, and how should the programmer fix them?
string find_addr( list<employee> emps, string name ) {
for( auto i = begin(emps); i != end(emps); i++ ) {
if( *i == name ) {
return i->addr;
}
}
return "";
}
Do not change the operational semantics of this function, even though they could be improved.
Informally, a temporary object is an unnamed object that you can’t take the address of. A temporary is often created as an intermediate value during the evaluation of an expression, such as an object created by returning a value from a function, performing an implicit conversion, or throwing an exception. We usually call a temporary object an “rvalue,” so named because it can appear on the “r”ight hand side of an assignment. Here are some simple examples:
widget f(); // f returns a temporary widget object
auto a = 0, b = 1;
auto c = a + b; // "a+b" creates a temporary int object
In contrast, in the same code we have objects like a and c that do each have a name and a memory address. Such an object is usually called an “lvalue,” because it can appear on the “l”eft hand side of an assignment.
That’s a simplification of the truth, but it’s generally all you need to know. More precisely, C++ now has five categories of values, but distinguishing them is primarily useful for writing down the language specification, and you can mostly ignore them and just think about “rvalues” for temporary objects without names and whose addresses can’t be taken, and “lvalues” for non-temporary objects that have names and whose addresses can be taken.
Believe it or not, this short function harbors three obvious cases of unnecessary temporaries or extra copies of objects, two subtler ones, and three red herrings.
The most obvious extra copies are buried in the function signature itself:
string find_addr( list<employee> emps, string name )
The parameters should be passed by const&—that is, const list<employee>& and const string&, respectively—instead of by value. Pass-by-value forces the compiler to make complete copy of both objects, which can be expensive and, here, is completely unnecessary.
Guideline: Prefer passing a read-only parameter by const& if you are only going to read from it (not make a copy of it).
Pedantic note: Yes, with pass-by-value, if the caller passed a temporary list or string argument then it could be moved from rather than copied. But I’m deliberately saying “forces the compiler to make a complete copy” here because no caller is realistically going to be passing a temporary list to find_addr, except by mistake.
Next we come to the first red herring, in the for loop’s initialization:
for( auto i = begin(emps); /*...*/ )
You might be tempted to say that this code should prefer to be spelled auto i(begin(emps)) rather than auto i = begin(emps), on the grounds that the = syntax incurs an extra temporary object, even if it might be optimized away. After all, as we saw in GotW #1, usually that extra = means the two-step “convert to a temporary then copy/move” of copy-initialization—but recall that doesn’t apply when using auto like this. Why?
Remember that auto always deduces the exact type of the initializer expression, minus top-level const and & which don’t matter for conversions, and so… presto! there cannot be any need for a conversion and we directly construct i.
So there is no difference between auto i(begin(emps)) and auto i = begin(emps). Which syntax you choose is up to you, but it depends only on taste, not on temporaries or any other performance or semantic difference.
Guideline: Prefer declaring variables using auto. Among other reasons to do so, it naturally guarantees zero extra temporaries due to implicit conversions.
Another potential avoidable temporary occurs in the for loop’s termination condition:
for( /*...*/ ; i != end(emps); /*...*/ )
For most containers, including list, calling end() returns a temporary object that must be constructed and destroyed, even though the value will not change.
Normally when a value will not change, instead of recomputing it (and reconstructing and redestroying it) on every loop iteration, we would want to compute the value only once, store it in a local object, and reuse it.
Guideline: Prefer precomputing values that won’t change, instead of recreating objects unnecessarily.
However, a caution is in order: In practice, for simple inline functions like list<T>::end() in particular used in a loop, compilers routinely notice their values won’t change and hoist them out of the loop for you without you having to do it yourself. So I actually don’t recommend any change to hoist the end calculation here, because that would make the code slightly more complex and the definition of premature optimization is making the code more complex in the name of efficiency without data that it’s actually needed. Clarity comes first:
Definition: Premature optimization is when you make code more complex in the name of efficiency without data that it’s actually needed.
Guideline: Write for clarity and correctness first. Don’t optimize prematurely, before you have profiler data proving the optimization is needed, especially in the case of calls to simple inline calls to short functions that compilers normally can handle for you.
Next, consider the way we increment i in the for loop:
for( /*...*/ ; i++ )
This temporary is more subtle, but it’s easy to understand once you remember how preincrement and postincrement differ. Postincrement is usually less efficient than preincrement because it has to remember and return its original value.
Postincrement for a class T should normally be implemented using the canonical form as follows:
T T::operator++(int)() {
auto old = *this; // remember our original value
++*this; // always implement postincr in terms of preincr
return old; // return our original value
}
Now it’s easy to see why postincrement is less efficient than preincrement: Postincrement has to do all the same work as preincrement, but in addition it also has to construct and return another object containing the original value.
Guideline: For consistency, always implement postincrement in terms of preincrement, otherwise your users will get surprising (and often unpleasant) results.
In the problem’s code, the original value is never used, and so there’s no reason to use postincrement. Preincrement should be used instead. Although the difference is unlikely to matter for a built-in type or a simple iterator type, where the compiler can often optimize away the extra unneeded work for you, it’s still a good habit not to ask for more than you need.
Guideline: Prefer preincrement. Only use postincrement if you’re going to use the original value.
“But wait, you’re being inconsistent!” I can just hear someone saying. “That’s premature optimization. You said that compilers can hoist the end() call out of the loop, and it’s just as easy for a compiler to optimize away this postincrement temporary.”
That’s true, but it doesn’t imply premature optimization. Preferring ++i does not mean writing more complex code in the name of performance before you can prove it’s needed—++i is not more complex than i++, so it’s not as if you need performance data to justify using it! Rather, preferring ++i is avoiding premature pessimization, which means avoiding writing equivalently complex code that needlessly asks for extra work that it’s just going to ignore anyway.
Definition: Premature pessimization is when you write code that is slower than it needs to be, usually by asking for unnecessary extra work, when equivalently complex code would be faster and should just naturally flow out of your fingers.
Next, we come to this:
if( *i == name )
The employee class isn’t shown in the problem, but we can deduce a few things about it. For this code to work, employee likely must either have a conversion to string or a conversion constructor taking a string. Both cases create a temporary object, invoking either operator== for strings or operator== for employees. (Only if there does happen to be an operator== that takes one of each, or employee has a conversion to a reference, that is, string&, is a temporary not needed.)
Guideline: Watch out for hidden temporaries created by implicit conversions. One good way to avoid this is to make constructors and conversion operators explicit by default unless implicit conversions are really desirable.
return "";
Here we unavoidably create a temporary (unless we change the return type, but we shouldn’t; see below), but the question is: Is there a better way?
As written, return “”; calls the string constructor that takes a const char*, and if the string implementation you’re using either (a) is smart enough to check for the case where it’s being passed an empty string, or (b) uses the small string optimization (SSO) that stores strings up to a certain size directly within the string object instead of on the heap, no heap allocation will happen.
Indeed, every string implementation I checked is smart enough not to perform an allocation here, which is maximally efficient for string, and so in practice there’s nothing to optimize. But what alternatives do we have? Let’s consider two.
First, you might consider re-spelling this as return “”s; which is new in C++14. That essentially relies on the same implementation smarts to check for empty strings or to use SSO, just in a different function—the literal operator””.
Second, you might consider re-spelling this as return { };. On implementations that are both non-smart and non-SSO, this might have a slight advantage over the others because it invokes the default constructor, and so even the most naïve implementation is likely not to do an allocation since clearly no value is needed.
In summary, there’s no difference in practice among returning “”, “”s, or { }; use whichever you prefer for stylistic reasons. If your string implementation is either smart or uses SSO, which covers all implementations I know of, there’s exactly zero allocation difference.
Note: SSO is a wonderful optimization for avoiding allocation overhead and contention, and every modern string ought to use it. If your string implementation doesn’t use SSO (as of this writing, I’m looking at you, libstdc++), write to your standard library implementer—it really should.
return i->addr;
return "";
This was a second subtle red herring, designed to lure in errant disciples of the “single-entry/single-exit” (SE/SE) persuasion.
I In the past, I’ve heard some people argue that it’s better to declare a local string object to hold the return value and have a single return statement that returns that string, such as writing string ret; … ret = i->addr; break; … return ret;. The idea, they say, is that this will assist the optimizer perform the ‘named return value optimization.’
The truth is that whether single-return will improve or degrade performance can depend greatly on your actual code and compiler. In this case, the problem is that creating a single local string object and then assigning it would mean calling string’s default constructor and then possibly its assignment operator, instead of just a single constructor as in our original code. “But,” you ask, “how expensive could a plain old string default constructor be?” Well, here’s how the “two-return” version performed on one popular compiler last time I tried it:
Note what this means: Not only did the single-return version generate slower code on this particular compiler on this particular day, but the slowdown was greater with optimizations turned on. In other words, a single-return version didn’t assist optimization, but actively interfered with it by making the code more complex.
In general, note that SE/SE is an obsolete idea and has always been wrong. “Single entry,” or the idea that functions should always be entered in one place (at their start) and not with goto jumps from the caller’s code directly to random places inside the function body, was and is an immensely valuable advance in computer science. It’s what made libraries possible, because it meant you could package up a function and reuse it and the function would always know its starting state, where it begins, regardless of the calling code. “Single exit,” on the other hand, got unfairly popular on the basis of optimization (‘if there’s a single return the compiler can perform return value optimization better’—see counterexample above) and symmetry (‘if single entry is good, single exit must be good too’) but that is wrong because the reasons don’t hold in reverse—allowing a caller to jump in is bad because it’s not under the function’s control, but allowing the function itself to return early when it knows it’s done is perfectly fine and fully under the function’s control. To put the final nail in the coffin, note that “single exit” has always been a fiction in any language that has exceptions, because you can get an early exceptional return from any point where you call something that could throw an exception.
Which brings us to the third red herring:
string find_addr( /*...*/ )
Because C++ naturally enables move semantics for returned values like this string object, there’s usually little to be gained by trying to avoid the temporary when you return by value. For example, if the caller writes auto address = find_addr( mylist, “Marvin the Robot” );, there will be at most a cheap move (not a deep copy) of the returned temporary into address, and compilers are allowed to optimize away even that cheap move and construct the result into address directly.
But what if you did feel tempted to try to avoid a temporary in all return cases by returning a string& instead of string? Here’s one way you might try doing it that avoids the pitfall of returning a dangling reference to a local or temporary object:
const string& find_addr( /* ... */ ) {
for( /* ... */ ) {
if( /* found */ ) {
return i->addr;
}
}
static const string empty;
return empty;
}
To demonstrate why this is brittle, here’s an extra question:
For the above function, write the documentation for how long the returned reference is valid.
Go ahead, we’ll wait.
Done? Okay, let’s consider: If the object is found, we are returning a reference to a string inside an employee object inside the list, and so the reference itself is only valid for the lifetime of said employee object inside the list. So we might try something like this (assuming an empty address is not valid for any employee):
“If the returned string is nonempty, then the reference is valid until the next time you modify the employee object for which this is the address, including if you remove that employee from the list.”
Those are very brittle semantics, not least because the first (but far from only) problem that immediately arises is that the caller has no idea which employee that is—not only doesn’t he have a pointer or reference to the right employee object, but he may not even be able to easily figure out which one it is if two employees could have the same address. Second, calling code can be notoriously forgetful and careless about the lifetimes of the returned reference, as in the following code which compiles just fine:
auto& a = find_addr( emps, "John Doe" ); // yay, avoided temporary!
emps.clear();
cout << a; // oops
When the calling code does something like this and uses a reference beyond its lifetime, the bug will typically be intermittent and very difficult to diagnose. Indeed, one of the most common mistakes programmers make with the standard library is to use iterators after they are no longer valid, which is pretty much the same thing as using a reference beyond its lifetime; see GotW #18 for details about the accidental use of invalid iterators.
There are some other optimization opportunities. Ignoring these for now, here is one possible corrected version of find_addr which fixes the unnecessary temporaries. To avoid a possible conversion in the employee/string comparison, we’ll assume there’s something like an employee::name() function and that .name() == name has equivalent semantics.
Note another reason to prefer declaring local variables with auto: Because the list<employee> parameter is now const, calling begin and end return a different type—not iterators but const_iterators—but auto naturally deduces the right thing so you don’t have to remember to make that change in your code.
string find_addr( const list<employee>& emps, const string& name ) {
for( auto i = begin(emps); i != end(emps); ++i ) {
if( i->name() == name ) {
return i->addr;
}
}
return "";
}
Thanks in particular to the following for their feedback to improve this article: “litb1,” Daan Nusman, “Adrian,” Michael Marcin, Ville Voutilainen, Rick Yorgason, “kkoehne,” and Olaf van der Spek.
Unnecessary and/or temporary objects are frequent culprits that can throw all your hard work — and your program’s performance — right out the window. How can you spot them and avoid them?
1. What is a temporary object?
2. You are doing a code review. A programmer has written the following function, which uses unnecessary temporary objects in at least three places. How many can you identify, and how should the programmer fix them?
string find_addr( list<employee> emps, string name ) {
for( auto i = begin(emps); i != end(emps); i++ ) {
if( *i == name ) {
return i->addr;
}
}
return "";
}
Do not change the operational semantics of this function, even though they could be improved.
This first problem highlights the importance of understanding what you write. Here we have a few simple lines of code—most of which mean something different from all the others, even though the syntax varies only slightly.
1. What is the difference, if any, among the following?
widget w; // (a)
widget w(); // (b)
widget w{}; // (c)
widget w(x); // (d)
widget w{x}; // (e)
widget w = x; // (f)
widget w = {x}; // (g)
auto w = x; // (h)
auto w = widget{x}; // (i)
2. What do each of the following lines do?
vector<int> v1( 10, 20 ); // (a)
vector<int> v2{ 10, 20 }; // (b)
3. Besides the cases above, what other benefits are there to using { } to initialize objects?
4. When should you use ( ) vs. { } syntax to initialize objects? Why?
This puzzle demonstrates several things:
But, most important of all: If you stick to two simple Guidelines, which we’ll cover in #4, you can ignore most of these cases and the rules are pretty simple and deliver efficient performance by default.
Let’s consider the cases one by one.
widget w; // (a)
This code declares a variable named w, of type widget. For most types, it is initialized using the default constructor widget::widget().
Note that w is not initialized and contains garbage values if widget happens to be a built-in type like int, or a simple “int-like” class type with what’s called a “trivial” default constructor—a type that relies on the compiler-generated default constructor, has no virtual functions or virtual base classes or data member initializers, and all its bases and members satisfy the same restrictions.
widget w(); // (b)
This is a pre-modern C++ pitfall: At first glance, it may look like just another variable declaration calling a default constructor widget::widget(); in reality, thanks to a grammar ambiguity, it’s a function declaration for a function named w that takes no parameters and returns a widget object by value. (If you can’t see this at first, consider that the above code is no different from writing something like int f(); which is clearly a function declaration.)
Lest you think “aw, but those ( ) parentheses are redundant, it’s the programmer’s own fault for not just writing widget w; there!”, note that the same problem arises in those occasional cases where you think you’re initializing a variable with temporary objects:
// same problem (gadget and doodad are types)
//
widget w( gadget(), doodad() ); // pitfall: not a variable declaration
Scott Meyers long ago named this “C++’s most vexing parse,” because the standard resolves the parsing ambiguity by saying: “if it can be a function declaration, it is.”
The good news is that this is now mostly a historical curiosity, not something you should encounter in new code, because C++11 removed this pitfall. Note that C++11 does not change the meaning of the code—C++11 has great backward compatibility with C++98, including that this ambiguity still has the same meaning it always had. Rather, C++11 solves this by providing a syntax that supersedes case (b) in nearly all cases, so that we don’t need to ever fall into this pit anymore:
widget w{}; // (c)
Here we have the first reason to prefer { } to ( ): For any class type widget, line (c) does the “best parts” of (a) and (b)—it always initializes the variable, and is never ambiguous with a function declaration. No vex, no fuss, no muss.
“Aha, but wait, it’s not that simple!” someone might object. “What if widget has a constructor that takes a std::initializer_list? Those are greedy (preferred), so if widget has one of those wouldn’t this call that?”
The answer is no, this really is just as simple as it looks, because the standard is explicit that an empty { } list means to call the default constructor if available. However, it’s good to be aware of initializer_lists, so let’s talk about those next.
Now let’s consider cases where we actually initialize w from some existing variable:
widget w(x); // (d)
widget w{x}; // (e)
Assuming x is not the name of a type, these are both direct initialization. That’s because the variable w is initialized “directly” from the value of x by calling widget::widget(x). If x is also of type widget, this invokes the copy constructor. Otherwise, it invokes a converting constructor.
However, note that the syntax {x} creates an initializer_list. If widget has a constructor that takes an initializer_list, that constructor is preferred; otherwise, if widget has a constructor that takes whatever type x is (possibly with conversions), that constructor is used.
There are two major differences that make (e) superior to (d): First, like (c), syntax (e) is unambiguous and avoids the vexing parse. If x is a type name, then (d) is a function declaration even if there is also a variable named x in scope (see above), whereas (e) is never a function declaration.
Second, syntax (e) is safer because it does not allow narrowing (a.k.a. “lossy”) conversions that are otherwise allowed for some built-in types. Consider:
int i1( 12.345 ); // ok: toss .345, we didn't like it anyway
int i2{ 12.345 }; // error: would be lossy implicit narrowing
This brings us to our final two non-auto cases:
widget w = x; // (f)
This is called “copy initialization.” Conceptually, the variable w is initialized using widget‘s move or copy constructor, possibly after calling another function to convert the argument implicitly (explicit conversions won’t be invoked here).
Common Mistake: This is always initialization; it is never assignment, and so it never calls T::operator=(). Yes, I know there’s an “=” character in there, but don’t let that throw you — that’s just a syntax holdover from C, not an assignment operation.
Here are the semantics:
Note that I said “conceptually” a few times above. That’s because practically compilers are allowed to, and routinely do, optimize away the temporary and, if an implicit conversion is available, convert (f) to (d), thus optimizing away the extra move operation. However, even when the compiler does this, the widget copy constructor must still be accessible, even if is not called—the copy constructor’s side effects may or may not happen, that’s all.
Now note the related syntax that adds “=“:
widget w = {x}; // (g)
This is called “copy list initialization.” It means the same as widget w{x}; except that explicit constructors cannot be used. It’s guaranteed that only a single constructor is called.
auto w = x; // (h)
auto w = widget{x}; // (i)
The semantics are just like (f) and (g), except simpler to teach, learn, and use because using auto guarantees the right-hand expression’s type will be deduced exactly. Note that the (i) syntax works fine for both implicit and explicit conversions.
Line (h) means the same as (d), type_of_x w(x);. Only a single copy constructor is called. This is guaranteed to stay true as the program evolves: Because line (h) does not commit to an explicit type, it is guaranteed to be both maximally efficient because there can be no conversion involved, and maximally robust under maintenance as the type of w “auto”-matically tracks the type of x which may change as the program is maintained.
Line (i) is the most consistent spelling when you do want to commit to a specific type and explicitly request a conversion if needed, and once again the { } syntax happily avoids lossy narrowing conversions. In practice on most compilers, only a single constructor is called—similarly to what we saw with (f) and (g), conceptually there are two constructor calls, a converting or copy constructor to create a temporary widget{x} followed by a move to move it to w, but compilers routinely elide the latter.
In general, I recommend that you try out these two forms, and increasingly prefer using them as you grow comfortable with them. I’m at the point where I’m now inclined to write virtually all of my local variable declarations this way. (I know some of you will be skeptical about this broad claim—more on “the auto question” in another GotW.)
In the Question 2 code, we’re creating a vector<int> and passing the arguments 10 and 20 to its constructor—in the first case as ( 10, 20 ) and in the second case as { 10, 20 }.
Both will call a constructor, but which one(s)? Well, vector<int> has several constructors that can take two parameters, but only two could be correctly called with the parameters 10 and 20. Ignoring defaulted optional allocator parameters for simplicity, the two constructors are:
vector( size_t n, const int& value ); // A: n copies of value
vector( initializer_list<int> values ); // B: copy of values
There are two simple C++ rules that tell us which one will be called for the code in question:
Armed with those two tidbits, the answer is simple:
vector<int> v1( 10, 20 ); // (a) calls A: 10 copies of the value 20
assert( v1.size() == 10 );
vector<int> v2{ 10, 20 }; // (b) calls B: the values 10 and 20
assert( v2.size() == 2 );
For one thing, it’s called “uniform initialization” because it’s, well, uniform—the same for all types, including aggregate structs and arrays and std:: containers, and without the “vexing parse” annoyance:
struct mystruct { int x, y; };
// C++98
rectangle w( origin(), extents() ); // oops, vexing parse
complex<double> c( 2.71828, 3.14159 );
mystruct m = { 1, 2 };
int a[] = { 1, 2, 3, 4 };
vector<int> v; // urk, need more code
for( int i = 1; i <= 4; ++i ) v.push_back(i); // to initialize this
// C++11 (note: "=" is mostly optional)
rectangle w = { origin(), extents() };
complex<double> c = { 2.71828, 3.14159 };
mystruct m = { 1, 2 };
int a[] = { 1, 2, 3, 4 };
vector<int> v = { 1, 2, 3, 4 };
And note that this isn’t just an aesthetic issue. Consider writing generic code that should be able to initialize any type… and while we’re at it, let’s gratuitously use perfect forwarding as an example:
template<typename T, typename ...Args>
void forwarder( Args&&... args ) {
// ...
T local = { std::forward<Args>(args)... };
// ...
}
forwarder<int> ( 42 ); // ok
forwarder<rectangle> ( origin(), extents() ); // ok
forwarder<complex<double>>( 2.71828, 3.14159 ); // ok
forwarder<mystruct> ( 1, 2 ); // ok because of {}
forwarder<int[]> ( 1, 2, 3, 4 ); // ok because of {}
forwarder<vector<int>> ( 1, 2, 3, 4 ); // ok because of {}
The last three lines would not be legal if forwarder used ( ) initialization syntax internally.
The new { } syntax works pretty much everywhere, including to initialize members:
widget::widget( /*...*/ ) : mem1{init1}, mem2{init2, init3} { /*...*/ }
And, as icing on the take, it’s often just plain convenient to pass function arguments, or return a value, without a type-named temporary:
void draw_rect( rectangle );
draw_rect( rectangle(origin, selection) ); // C++98
draw_rect({ origin, selection }); // C++11
rectangle compute_rect() {
// ...
if(cpp98) return rectangle(origin, selection); // C++98
else return {origin, selection}; // C++11
}
Here’s the simple guideline:
Guideline: Prefer to use initialization with { }, such as vector<int> v = { 1, 2, 3, 4 }; or auto v = vector<int>{ 1, 2, 3, 4 };, because it’s more consistent, more correct, and avoids having to know about old-style pitfalls at all. In single-argument cases where you prefer to see only the = sign, such as int i = 42; and auto x = anything; omitting the braces is fine. …
That covers the vast majority of cases. There is only one main exception:
… In rare cases, such as vector<int> v(10,20); or auto v = vector<int>(10,20);, use initialization with ( ) to explicitly call a constructor that is otherwise hidden by an initializer_list constructor.
However, the reason this should be generally “rare” is because default and copy construction are already special and work fine with { }, and good class design now mostly avoids the resort-to-( ) case for user-defined constructors because of this final design guideline:
Guideline: When you design a class, avoid providing a constructor that ambiguously overloads with an initializer_list constructor, so that users won’t need to use ( ) to reach such a hidden constructor.
Thanks in particular to the following for their feedback to improve this article: Michal Mocny, Jay Miller, “Alexey,” “praetorian20,” Francisco Lopes, “Neil,” Daryle Walker.
