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.
Problem
JG Question
1. What makes interfaces “easy to use correctly, hard to use incorrectly”? Explain.
Guru Question
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.
Solution
1. What makes interfaces “easy to use correctly, hard to use incorrectly”? Explain.
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.
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?
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:
1. The default constructor is missing.
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.
2. operator+ passes by value.
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).
3. operator+ modifies this object’s value.
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.
4. operator+ is not written in terms of operator+= (which is missing).
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.
5. operator+ should not be a member function.
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.
6. operator<< should not be a member function.
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.
7. operator<< should return ostream&.
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>>.
8. The preincrement operator’s return type is incorrect.
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.
9. Postincrement should be implemented in terms of preincrement.
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.
Summary
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);
}
Acknowledgments
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.
Sutter’s Mill 
@Luca Risolia:
I you want to consider all r/l-value cases you would need to implement all four
T operator+(const T& a, const T& b)
T operator+(T&& a, const T& b)
T operator+(const T& a, T&& b)
T operator+(T&& a, T&& b)
This’s overkill, so it’s probably a good idea to concentrate on the most-used form, which is the temporary in the first argument for chains of operator +. For other classes, e.g. strings, this is all that’s ever needed, since operator + is not symmetric.
@Zenju
complex a; auto r = operator+(a, complex{});a is a lvalue while the second argument is a temporary, which means operator+(const complex&, complex&&) will be called. You can easily have more convoluted expressions in which the same overload might be called. Associativity rules do not change the result for a complex type. Do I miss anything?
@Luca Risolia: operator + has left-to-right associativity, so the first parameter will have to deal with temporaries, not the second.
> 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.
While talking about optimizations, it should be mentioned that passing one parameter by value in order to “unify” l/r-value parameters is not the most efficient solution:
In case the first parameter is a temporary, there is a move-construction that could otherwise be avoided had the function been specified as:
T operator+(T&& a, const T& b )
So for maximum efficiency it is best to have these two overloads instead:
T operator+(const T& a, const T& b) and
T operator+(T&& a, const T& b)
This might seem nit-picky, but in general move-construction is not free! For example I know a string class which always has an associated control structure(alloc-count, size, ect.). Due to implementation details, the destructor cannot simply call “operator delete”, but requires a valid control structure.
Therefore even the move-constructor must create a dummy constrol structure, which boils down to a memory allocation, i.e. a system-call!
@Herb, I apologize if this has been already suggested by others, but what about recommending the following version of
other than the one you propose in the final version of the complex class:
complex operator+( const complex& lhs, complex&& rhs ) { rhs += lhs; return rhs; }Compared to the original solution, this way an extra-copy can be avoided when a temporary object is passed as argument to rhs and the const ref lhs is bound to an lvalue.
Thanks for that answer, Herb.
I was searching for an example where the difference between the two different forms of parameter passing may matter, and the only example I could think of is where the class must satisfy some imposed API. Inheriting from an abstract base class is the most obvious example; I haven’t put any thought to whether or not a concept could do it.
Hiding the implementation detail by removing the call-by-value is EXACTLY the same thing as hiding the implementation by introducing a layer of abstraction or indirection (e.g. inheriting from an ABC, or implementing the class using a pimpl). That’s the bit I was missing.
@decourse: f( int ) and f( const int& ) tell the caller the same thing: He provides an int as the first argument, and his int won’t be modified. You ask, why is that in the signature where the caller can see the implementation detail (and must recompile if it changes), even though it doesn’t affect him?
So now consider:
class widget { /*...*/ private: int x; };Guru question: Why is x in the class definition where the caller can see that implementation detail (and must recompile if it changes), even though he can’t use it?
The two questions are the same. The reason it is necessary to surface this difference in the function signature, even though the caller of the function can’t be affected by it and doesn’t care, is the same reason why we have to make private class members visible in the class definition, even though the caller of the class can’t use them and doesn’t care: Link compatibility requires the compiler knows the calling convention incl. parameter passing and the full layout of all objects used as a value. Even though the caller cannot use such “private” information, it must always be available to the compiler on both sides. The fact that the compiler always knows this is also one of the reasons C++ is a naturally highly optimizable language.
@Herb, I guess what I’m after here is some advice as to what passing an object by value actually means.
We all know by now the difference between passing a smart pointer and passing a raw pointer. In each case, choosing one over the other signals a clear intention about the interface. I the signature asks for a smart pointer, for example, it “means” that the function wants to manage the lifetime of the passed object, but if it asks for a raw pointer, that “means” it doesn’t. It’s a similar story with lvalue references and rvalue references. Moreover, these differences are part of the interface.
I don’t really know what it means when a function asks a caller to pass an object by value. But whatever it means, it’s not really part of the interface, but part of the implementation.
I don’t like that. It smells wrong.
@Matthew: Yes, there will eventually be another GotW on parameter passing that will cover these kinds of things.
@Herb:
Trying to code based on whether or not the optimizer can do RVO on named temporaries or only unnamed return values is a distraction I agree.
When it comes to value types however, there are no move semantics so the optimization game we’re trying to play here is copy ellision in either one of the parameters or the return value. The problem is that with passing one of the parameters by value is that the optimization only works if the caller happens to use an rvalue for that parameter (5 + x) vs (x + 5).
For something like operator+(), we always use the return value so copy ellision can always be taken advantage of if its done on the return side vs on the parameter side. This could matter for a math library such a 3d linear algebra library for a game engine where you are doing a lot of 4×4 matrix multiplies. Theres also the minor issue that the copy for passing by value is done at every call site and possibly bloats the code if the function is called often.
Also when passing by value, which argument should be a reference and which a value?
Should we pick a convention and suggest everyone follow it? Much like preferring ++i to i++ with iterators? I often find myself writing x + 5, than I do 5 + x.
If your type has move semantics then the most correct way to do it is with a bunch of ugly overloads.
This is probably another GOTW, but I’d really like to know if there is any guidance for how to reduce the ammount of code here:
Perhaps the following:
T operator+(const T& a, const T& b) { T c = a; //this one must do a copy c += b; return c; } T operator+(const T& a, T&& b) { T c = std::move(b); //great, we can move c += a; return c; } T operator+(T&& a, const T& b) { return b + std::move(a); //calls lv + rv } T operator+(T&& a, T&& b) { return a + std::move(b); //calls lv + rv }Or this? (Does this even work like the above in all cases?)
T operator+(T a, const T& b) { return a += b; //lv + lv or rv + lv } T operator+(const T& a, T&& b) { return std::move(b) + a; //calls rv + lv } T operator+(T&& a, T&& b) { return std::move(a) + b; //calls rv + lv }@Herb Sutter (2013-5-21T10:59): I’ve heard about transcendental functions being implemented with the help of internal constant tables. Constexpr can help there. A response by Róbert Dávid described something similar. The constexpr is also a just-in-case some programmer changes complex to a class template, like the “value_type” alias added. (Like if we want “int” versions. There’s no reason for complex to be limited to floating-point; anything besides division, “abs,” and the transcendentals works just fine with integer arithmetic.)
What the haters don’t see is that “superfluous” code isn’t over-engineering basic stuff, but adding the finishing details. They’re refinements. Middle programmers have to worry about those details and do a pass beyond make-it-work so they don’t pass inefficiencies down the line.
Maybe you can do a GotW on constexpr someday.
@Róbert Dávid (2013-5-21T12:22): Yes, it could be considered overkill, but complex range-for support may come in handy for generic code. The tuple support is another example.
can somebody explain what Herb meant with this: ” That makes the class annoying to use at all.”
@Daryle Walker: I’m not sure it is a good idea to use range-for on a.. single.. complex value.
@Herb Sutter: Compile-time float values is something I would kill for while implementing code from mathematicians, containing code that is essentially possible to do compile time – but it is not feasible to just code the ‘result’ into the code, as the constant changing every iteration. To oversimplify, something like “return input*2*sqrt(3)” at first, “return input*3*sqrt(3)” in the second version, “return input*3*sqrt(5)” in the third (mixed with changes of ‘non-compile-time’ logic as well). And this happened in three companies so far, so I have a sense that this isn’t really uncommon. Generally, I would like to push as much computation to compile time as possible – constexpr can do miracles, even with its current, limited version. (constexpr dgemm, anyone?)
Sorry that was obviously supposed to be:
T operator+( const T& lhs, const X& rhs ) { T rv = lhs; rv += rhs; return rv; }While pass by value for things like operator is good in theory. I’m not sure how good it is in practice. There was recently talk about this related to the boost operators library.
Compare
T operator+( T lhs, const X& rhs ) { lhs += rhs; return lhs; }http://ideone.com/q1ETx3
T operator+( const T& lhs, const X& rhs ) { T rv = lhs; rv += lhs; return rv; }http://ideone.com/wutk1x
@Daryle: I actually did consider whether to talk about making this constexpr, and I deliberately decided not to (and almost mentioned in this in the summary alongside why I didn’t talk about final). Two reasons:
(1) I can’t think of any use cases for a compile-time complex number (or any floating point value for that matter); it’s not like we’re going to initialize arrays with them or set enumerator values with them or instantiate templates with them, right? I could be wrong here — does anyone know of a valid constexpr use for a floating point number?
(2) Doing it would make this simple class more complicated, as I think you’ve demonstrated. :) And the extra work is for no real advantage if I’m right about (1), and with the disadvantage of providing incorrect fodder for people looking for disinformational examples to claim how hard simple stuff is in C++, when it really isn’t (at least not in this case).
I think we should be teaching how simple it really is to write high-quality C++ code, not how complex C++ code can be made by people who revel in complexity (which we’re all guilty of from time to time of course). This example really is simple and doesn’t need constexpr, so I didn’t even mention constexpr. If I’m wrong about that, I’d really be happy to know — what are the use cases for making complex constexpr-aware? On the other hand, if I’m right about that but the question is a FAQ, maybe I should add this kind of note about constexpr in the Summary along with why I didn’t mention or recommend saying final either.
@decourse: It’s not an abstraction leak because it doesn’t affect the caller’s argument. It’s an implementation detail that happens to be visible but isn’t a “leak” inasmuch as it has no effect on the calling code.
@Marshall and Matt: Even in the 1990s, my impression was that talking about writing your code in order to try to enable (N)RVO was overdone; optimizers have been good at this for a long time, some did better with a named temporary and others with an unnamed return value, and debates about whether a named local variable or reusing a parameter or creating a temporary would help the optimizer the most were generally a distraction. In general I think we really do want to teach people to: (a) write code for clarity first (overthinking (N)RVO is usually asking for premature optimization), and (b) pass an input value parameter by value if you’re going to copy from it anyway, because it really does help when passed rvalues. The community seems to be converging on that this is the right advice.
@hm: Thanks, that “explicit” was a leftover from the original 1997-vintage version that no longer applies since I’ve updated the advice (and don’t even talk about explicit conversions any more in the current version). Removed.
@Dave Harris: Thanks, “for” was missing. Added.
Point 8, “Ignoring the sake of argument”, looks like an editing error. I think you mean, “Ignoring the argument” and were thinking of “For the sake of argument”. Not important, but probably should be fixed if those words might make it into print.
Why did I change the component implementation from two value_type objects to an array of two objects?
1. Why not? Especially since C++11 allows arrays to be put in the member-initialization list
2. It’d only be a problem if the member(s) were directly exposed, but we’re hiding them. Your implementation for the accessors can change to match how the data is stored.
3. The components are guaranteed to be packed, so since the class is also standard-layout, we can reinterpret between a complex and an array-of-two-doubles for some kinds of C-level fun. (We can’t do array-of-complex and array-of-twice-the-doubles reinterpretations unless we can guarantee no rear padding somehow.)
4. We can do this:
class complex { public: //... friend constexpr auto begin( complex const &c ) noexcept -> value_type const * { return &c_[0]; } friend constexpr auto end( complex const &c ) noexcept -> value_type const * { return begin(c) + 2; } // Do the non-const versions, maybe with const_cast... friend void swap( complex &a, complex &b ) noexcept(???) { std::swap(a.c_, b.c_); } //... };And now it will automatically work with range-for! (The swap isn’t there for any reason but to show the new routine in the standard for built-in arrays.) Heck, we can add tuple support:
class complex { public: //... template < std::size_t I > friend auto get( complex const &c ) noexcept -> value_type const & { static_assert( I < 2u, "Index out of bounds" ); return c.c_[ I ]; } template < std::size_t I > friend auto get( complex &c ) noexcept -> value_type & { return const_cast<value_type &>(get<I>( const_cast<complex const &>(c) )); } template < std::size_t I > friend auto get( complex &&c ) noexcept -> value_type && { return std::forward<value_type>(get<I>( c )); } //... }; //... namespace std { struct tuple_size<my::complex> : integral_constant< size_t, 2u > { }; template < size_t I > struct tuple_element< I, my::complex > { static_assert( I < 2u, "Index too large" ); using type = my::complex::value_type; }; }>> You don’t need to create a temporary in the return statement for RVO, at least not on modern versions of gcc.
I’ll take your word for it – but there are other compilers in the world besides gcc.
Besides, the more I look at that code, the more I like it – it says very clearly what the routine does: “Return a new complex object whose value is the sum of the two parameters ‘a’ and ‘b'”
Besides having the arguments in the wrong order, and returning void, the output routine has another problem. Not only is taking (and returning) references to streams is suggested, it’s required! Streams are explicitly not copyable. (This was uncertain in C++03, but made definitive in C++11.)
The stream classes are now templates, making them unsuitable with stuff marked virtual. You would either have to ignore any version of streams besides char, or set up a complex network that works around the virtual/template boundary.
I disagree with a “print” function for another reason. It’s duplicitous; it does the EXACT SAME thing as the stream operator. Why would you ever need both? Can you think of a situation that you would need “print” instead of the operator? Users might get confused, think that there’s some subtle difference, but there wouldn’t be one. You don’t need “print” for access; the stream operators should work on publicly accessible data, resorting to friend access if there’s not enough getters. The output streamer should print enough public information that the input streamer can read it back and create/construct an equivalent copy.
class complex { public: //... template < typename Ch, class Tr > friend basic_ostream<Ch, Tr> & operator <<( basic_ostream<Ch, Tr> &o, complex const &c ) { return o << '(' << c.c_[0] << ',' << c.c_[1] << ')'; } //... };You shouldn’t actually implement the output-streamer this way; the width modifier would apply only to the first section (the “(“) and spew everything else to the stream as-is. Like std::complex, stream the five sections to a string then print the string.
@Marshsall:
You don’t need to create a temporary in the return statement for RVO, at least not on modern versions of gcc. Creating a temporary on the stack and returning it later works just fine.
In my previous post, note that the component-wise constructor is not explicit. That is intentional. Explicit constructors are for configuration parameters, while implicit ones are for equivalency (either conversion and/or component-wise). It’s like std::vector: the size-setting constructor is explicit, while the iterator-pair and initialization-list ones aren’t. In C++03, the decision for making constructors explicit only mattered for ones with a single-argument mode; now it’s for all constructors. We moved that model to multivalued initialization, whether by std::initialization_list, variadic arguments, or regular constructors with several arguments.
Another way to look at it is that complex numbers are a natural upgrade for real numbers, so having a non-explicit converting constructor makes sense.
I have a separately-defined default constructor, marked “default.” I could have incorporated it into the component-wise/converting constructor, but I wanted distinct semantics. When the value_type is trivial (and since it’s double, it is), this constructor upgrades the class from trivially-copyable to trivial. This lets default-set, but not value-initialized, complex objects have random garbage bits. This is a GOOD thing. The real and imaginary components are independent; there’s no invariant on either for the class or each other. Letting complex be a trivial type means it can play around with C-level routines.
The complex class is also standard-layout. This allows other kinds of C-level play. Since I changed the components from being separate members to a two-element array, I can exploit the same complex array-of-2 reinterpretations the standard wants for std::complex. (The standard expects the reinterpetation to work for an array-of-complex array-of-twice-the-reals, but I can’t do that unless there’s no trailing padding. That can only be done if the compiler supports a Pragma forcing the lack of padding.)
The complex class is POD since it’s both trivial and standard-layout.
The default constructor can’t be constexpr since it’s set to be trivial. A constexpr constructor has to be code-full for use in constant expressions, but the point of a trivial constructor is that it’s code-less!
Your code doesn’t take constexpr into account, which changes quite a few things.
class complex { public: // Only have to change the component type in one spot using value_type = double; // Constructors complex() = default; constexpr complex( value_type const &r, value_type const &i = value_type{} ) : c_{ r, i } { } //... private: // Member data value_type c_[ 2 ]; };First, you need a constexpr-marked constructor, and the component-wise one will do quite nicely. Fortunately, you can initialize arrays in the member-initialization part now.
The advice to implement OP in terms of OP= is somewhat bad now due to constexpr. Implementing a non-mutating function in terms of a mutating one disqualifies the former from being constexpr (since the latter can’t). You either have to reverse the dependencies or write them independently:
class complex { public: //... complex & operator +=( complex const &a ) { c_[ 0 ] += a.c_[ 0 ]; c_[ 1 ] += a.c_[ 1 ]; return *this; } // Has to be a friend if you don't have component-level getters friend constexpr complex operator +( complex const &l, complex const &r ) { return {l.c_[0] + r.c_[0], l.c_[1] + r.c_[1]}; } //... };I was going to go the reversed-dependency route, but I had a epiphany about the “independent” route. The two operators look unconnected, but notice that each one is implemented in terms of the corresponding element-level operator. If the element-level operators are connected, then the current ones will be indirectly connected. So the old advice is like a hobgoblin’s foolish consistency (or something).
[I’ll have some more posts, assuming this code.]
You did not explain why to make the ctor explicit (but you did it explicit in the summary code).
Can someone explain to me why this is not an undesirable abstraction leak?
I think it’s defensible in this case, especially since operator+ is trivial. In the general case, I think it’s bad advice to leak information about the way a function is implemented into its interface.
To enable RVO, I would be tempted to write this:
T operator+(const T& a, const T& b) { return T { a.r + b.r, a.i + b.i}; }T operator+( T a, const T& b ) { a += b; return a; }[quote]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.[/quote]
In some cases it may actually be better to pass both arguments by reference so that the compiler can take advantage of return value optimization. For simple value types like complex, a 3d vector, or a 4×4 matrix (where the data is an array the class, not dynamically allocated), this is better because there are no move semantics to take advantage of.
T operator+(const T& a, const T& b) { T c = a; c+=a; return c; } Also you can't be fancy and try to do this: [code] return c+=a;Now the return value is a reference from operator+=() instead of the temporary you created and the compiler may no longer be able to do RVO.
Thank you so much for this article. I see so many who make some of these common errors every day, particularly making operators member functions and not taking the first parameter by value in cases like operator+. If they just discovered this article, they would be much better off.
Anyway, I believe you have a small typo:
“For example, if you provide operator++ you would normally also provide operator–.”
These two don’t always match (think: bidirectional iterator, think harder: forward iterator). You can argue that those aren’t the general case, but I would probably go with + and – (even specifying unary or binary if you want).
@litb1: Good point, that’s worth mentioning. Added.
I wonder whether there is any way to avoid the redundancy in the default argument for imag and the inclass initializer for it (when defaulting the default constructor). One alternative is constructor delegation
complex ():complex (1.0f) {}@Marshall: Re const, it doesn’t quite mean that — wait for GotW #6a. Re aliasing (other parameters): Yes, that’s another advantage of pass-by-value. Re move from rvalues, yes, that’s part of what I added in the text I added in response to Olaf’s comment.
You mention that as an alternative the following can be used
complex () = default;
Wouldn’t this leave the two non-class data members uninitialized (as they are default initialized)?
@Olaf: Thanks. I agree, this is a good place to mention the new C++11 guidance to pass by value if you’re going to make a copy of the parameter anyway. Updated, including not just there but also in the canonical operator+ with a new paragraph and guideline there, so that now we have both of these guidelines in the text:
Guideline: Prefer passing a read-only parameter by const& if you are only going to read from it (not make a copy of it).
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.
I’ve also updated the wording of the first of these guidelines in GotW #2 to match this wording.
Note that the new paragraph alludes to my intention to write a new GotW about parameter passing… “how to pass parameters” deserves a GotW all its own, because the C++11 answer is simultaneously different, considerably simpler in the mainstream, and considerably more complex when you want to take full control, than the C++98 answer.
(a) Regarding #2 (and given the above comment by Marshall, specifically the aliasing part) — I’m still wondering, wouldn’t pass-by-value be a better fit here, per N3445?
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2012/n3445.html
Specifically:
“We can change our habits to pass trivially copyable classes of sixteen bytes or less by value. With this approach, complex numbers would primarily be passed by value, not by reference as they are in the class template complex (26.4 Complex numbers [complex.numbers]). This approach avoids aliasing issues, enables removing some indirect references when accessing parameters, but may introduce copying on older platforms.”
I’m guessing this may have something to do with the latter part, stating that the “net performance change is unclear.” However, isn’t this case similarly unclear for pass-by-value for our specific class (given its size and the presumed use for numerical computing, where aliasing might have an important impact on performance)?
I’m kind of interested in this issue personally and somewhat playing the Devil’s advocate here, since so far I’ve been mostly using pass-by-const-reference for all (strictly-)larger-than-scalar types, just want to make sure I’m not missing a potentially important edge case by not expanding this to “small classes” (as in N3445).
(b) Regarding prohibiting inheritance from value types being an overkill — perhaps, but isn’t it precisely addressing the “easy to use correctly, hard to use incorrectly” point?
I’m thinking in terms of the Standard Library containers here (perhaps that’s too much of an association, correct me if I’m wrong) — is it *ever* justified to inherit from one of those?
>> Guideline: Prefer passing read-only parameters by const& instead of by value.
There’s a subtlety here, new in C++11.
* What we’re used to saying is that “passing by const &” means the value will never change.
* What we have now is that “passing by const &” means that the callee will not change the value. Other threads of execution may do so – at any time.
This can lead to lost optimization opportunities, because the compiler cannot be sure that “other.i” (say) will not change during the execution of operator+. [ I’m speaking generally here, b/c in this case I don’t think it matters. ]
If you pass by value, then the compiler can (a) know that this value is unique (i.e, no other pointers or references alias it), and (b) use move semantics when appropriate.
complex operator+( const complex& lhs, const complex& rhs ) {What about this?
complex operator+( complex lhs, const complex& rhs ) { lhs += rhs; return lhs; }In #5 I think you meant “the other reason to prefer non-members” not “the other reason to prefer members”.