Comments for Sutter's Mill
Comment on GotW #4: Class Mechanics (7/10) by Bjarke Hammersholt Roune
Good interface design is an exercise in tradeoffs. There is no single meaning of something like "easy to use correctly, hard to use incorrectly" – like everything in life, it depends on context. This is a list of things to consider. It is a rare situation indeed where there is any way to do all of these things at the same time. It’s a trade-off. In many cases, some of these items can’t be done no matter what you do.
* If the main concern for your program is performance, and the interface will participate in performance-critical computations, then avoid designing an interface that makes it difficult or impossible to achieve top performance. In this case, correct use of an interface implies efficient use. For example, avoid unnecessary copying and memory allocation when possible.
* Make method names succinctly, aptly and clearly describe what the method does. I know what obj.setX(5) probably does, but I’m much less sure about obj.adjustX(5). Use long method names if that is necessary.
* Follow the conventions of the code that the interface is a part of.
* Avoid abbreviations unless you are sure that the user of the interface will understand them easily. Abbreviating pointer to ptr is probably fine. Abbreviating “the identity function” to “id” is only OK if you know for a fact that everyone who will have to deal with your interface is going to make the connection easily.
* If clearly describing what a method does requires a 30+ character name, try to see if you can’t change what the function does so that you can give it a shorter name. Perhaps split it into two simpler methods.
* If a method is unavoidably so complicated that a user of it simply must read the documentation, give it a name that will trigger people to go look at the documentation. Don’t let obj.setX(5) do something complicated – I’m going to think that I know what it does and I’ll be wrong. Rename it to something complicated or mysterious (yet still apt) so people will go look up its documentation because they realize that they can’t figure out what that means without looking at the documentation. First, try to redesign the interface so you don’t need that method in the first place.
* Consider how your interface will show up in intellisense and compiler error messages. For example, if you have a public foo(int) and a private foo(MyInternalType), users of your interface are going to see the private foo in error messages and in intellisense, which probably isn’t what you want. Give the private foo a different name (oh, and don’t name your methods foo).
* Avoid methods with many parameters of the same type. If you’ve got a function like foo(bool, bool, bool, bool), then likely each parameter does something different and it will be difficult for people to see at a glance whether the order of the parameters is correct. Sometimes there’s no good way around this, but a possible replacement would be fooA(bool), fooB(bool), fooC(bool) and fooD(bool), where A, B, C and D are descriptions of what those bools do.
* Avoid designing an interface where obj.doSomething(false) causes something to be done. For example, doSomething(bool) might do what it does in a different way depending on the parameter. This is confusing. It looks like you are asking for something not to be done.
* Suppose that you know that most of the people who will be using the interface do not understand some advanced concept, like template-templates in C++ or topologically sorted directed acyclic graphs. If you must use such a concept anyway, you need to document the concept carefully, perhaps by offering a link to an online resource. First, try to avoid using the concept.
* Minimize preconditions. For example, suppose you take a std::vector& as a parameter and that you need this vector to be sorted and that you need to hold on to the vector for a while. You could have a precondition stating that the vector must be already sorted and that it must stay alive and be unchanged for the life-time of your object. Instead, you could simplify the precondition by copying the vector and then sorting it yourself.
* Avoid potentially bad interactions between the methods in the interface. If every call to foo() must be followed by a call to bar(), and if something bad happens if you do it the other way around, then that makes the interface harder to use correctly. There’s often no good way around this.
* Design the interface so that it is possible to follow the Liskov substitution principle. From Wikipedia: “if S is a subtype of T, then objects of type T may be replaced with objects of type S (i.e., objects of type S may be substituted for objects of type T) without altering any of the desirable properties of that program (correctness, task performed, etc.).”
* Minimize visible state in your interface. The dynamic type of an object is one example of a state. That kind of state is accessed through virtual functions and the Liskov substitution principle is essentially stating that the client code should not have to know the sub-type. More generally, it is desirable for an interface not to have any state at all, or if it does have state, then client code should ideally not have to worry about what the state is. Some kinds of state are not a problem, for example because it is very simple (setName() and getName()) or because it’s not visible (a transparent cache).
* If your interface participates in parallel computation, make it very clear what the contract for parallel use of the interface is.
* Design the interface so that it is possible to use the type system to check for bugs. You do that by making as many bugs as possible cause compile-time errors or at least warnings. You could try to expand this by making your interface easy for static analysis tools to understand, perhaps by annotating it.
* Design your interface so that there is no tricky or repetitive thing that the user of the interface has to remember to do. Returning a pointer that has to be deleted is a great/horrible example of that (use a smart pointer). Returning an error code that must be checked is another example (use exceptions).
* Offer exception guarantees. Offer the strong exception guarantee when you can. If you can’t, offer the weak one. Document what guarantee you are giving – maybe not always, but at least in all cases where the answer is not what one would expect.
* Don’t design an interface that can’t be implemented without using global state. (Singletons are global state).
* Design the interface so that it is possible for the implementation of the interface to check that it is being used correctly at run-time. For example, if the parameter t to obj.foo(t) must be less than some number X, then you could make it mandatory to inform obj about X, so that t can be checked inside foo, even if it would not otherwise be necessary to know the value of X to implement the interface.
* Make it testable. If you can’t test to the interface in a reasonable way, there’s more likely to be bugs that you won’t be aware of until the worst possible time. It’s hard to use an interface correctly if it wasn’t implemented correctly.
* Encapsulate. The interface should not reveal how it is being implemented.
* Don’t do too much. An interface with too many responsibilities tends to be hard to use.
* Make it general. You want to be able to use this interface in different contexts, so don’t make your interface specific to a context for no reason.
* Do something well. If you make your interface too general, you might end up with something that will work in every relevant circumstance, but that doesn’t fit well to any specific use. Make sure your interface is a good fit in at least one situation.
* Consider the impact of your interface on the whole program’s complexity – both statically and at run-time. It is sometimes possible to make an interface really simple and nice-looking, but programs that use that interface end up having to build very complicated structures in memory. For example, the observer pattern is simple (and sometimes a good choice), yet you can use it to create an impossibly complex maze of calls-backs at run-time. If your interface encourages or even requires that sort of thing, then that makes it harder to use correctly. Sometimes it’s better to complicate your interface if the result is that client code ends up constructing simpler structures in memory. Pushing complexity from your interface into the surrounding code or into the run-time characteristics of the program isn’t always a good idea.
Read More »
Comment on GotW #4: Class Mechanics (7/10) by rbrown
To those suggesting that the constructor be made explicit, it’s worth noting out that std::complex has a constructor of the form (note the lack of an “explicit” keyword):
constexpr complex(double re = 0.0, double im = 0.0);
As litb1 pointed out, making the constructor explicit will prevent the use of a C++11 initializer list for copy-initialization. For example, the following won’t compile:
void f(complex c); f({ 1.0, 1.0 }); f({ 1.0 })
I think in this case an implicit conversion is natural, and I don’t see it leading to unintended side-effects (unlike, say, going from std::string to const char *).
Read More »
Comment on GotW #4: Class Mechanics (7/10) by qdii
@Crazy Eddie
I disagree with your idea of adding getters to this class.
A complex number can be defined in different ways, one of them being the sum of a and ib, as it is here. Another implementation could rely on its absolute value and its argument.
To make an example, consider a complex class implemented in term of absolute value and argument:
class complex { public: double getReal() const { return /* formula */; } double getImaginary() const { return /* formula */; } private: double argument; double absoluteValue; };
Now what would operator==() code be?
bool operator==(const complex& a, const complex& b) { return (a.getReal() == b.getReal()) && (a.getImaginary() == b.getImaginary()); }
This probably won't work because of round-up errors, and even if it does, it will under-perform because of all the computation needed by getReal() and getImaginary(). This is why operator== needs access to the inner guts of the class, in a perfect world it would really be part of the class, but for design reasons (namely, symmetry), it was cast away.
Read More »
Comment on GotW #3 Solution: Using the Standard Library (or, Temporaries Revisited) by Leo
Herb, if Bjarne’s expectations for C++14 (https://www.informit.com/articles/article.aspx?p=2080042) are fulfilled wouldn’t the STL provide an overload like the following (if it would work):
template<Container Cont, Predicate Pred> requires Input_iterator<Iterator<Cont>>() Iterator<Cont> find_if(Cont& c, Pred p);
If then the fact could be taken into account that a Predicate is not supposed to change its argument, the type of e could default to const auto&. Then one could write:
const auto i = find_if(emps, [&](e) { return e.name() == name; });
Read More »
Comment on GotW #3 Solution: Using the Standard Library (or, Temporaries Revisited) by Roger
With lambdas now available, I use the built in functions as much as I can. I often find I cannot because relationships between elements in a list are often important (remember the index to something, or whatever).
But the code is still so frustrating to type, and ugly. We have ranges now, wouldn’t it be nice to add it into the standard library for all things that take ranges? We also have auto, it would be nice if we didn’t have to keep typing the names of types that we already know. Consider:
const auto i = find_if (e:emps, [&](auto) {return e.name() == name});
I’m sure that syntax would break something in C++, but this is along the lines of what I want in a modern language. Herb said something in a talk about C++11 not being your dad’s C++ (or a metaphor to that effect). Well, it still is, albeit better. I don’t want to type begin(x), end(x) anymore. I don’t want to type types for a lambda when they can be figured out. More importantly, I don’t want to parse through tons of boilerplate code that somebody else typed (or that I typed 6 months ago) to try to figure out what is being done.I find myself moving back to using simple for loops, I find them clearer to read and a heck of a lot easier to type. I don’t want to have to keep in my mental stack the iterator that the find_if introduces. I don’t want to have to parse the ? operator code to see which value is returned in each case. Range based for is readable, the last solution is just a mess.
Actually, i would love to write something like:
string find_addr (emps,name) { return e.addr if e.name == name in e:emps; return ""; }
and just have the compiler figure out things like name can be passed by const reference, that emps needs to be something that supports ranges and has addr and name fields, and that the auto is implied. That’s true information density.
C++ has become so unreadable. ‘const std::shared_ptr<std::vector>&’ is a typical signature for a single parameter in my code. (I would love a GotW that focused on using managed pointers in a readable way, btw).
Sorry if this comes off as a rant, but I think conciseness and readability is far more important than removing any possible creation of a temporary variable, and more important than following a (generally good) rule like use the standard library. Try reading code where a default parameter like less is used, and search through all the headers, and headers included by headers, to figure out what version of less is being used by the standard library call for a user defined struct. A for loop makes it all explicit, often with far fewer characters. Explicit is good. Readability is good. I would not want my engineers writing code like Herb’s last example unless this function was so critical that efficiency came before anything else. It’s horrible, ugly code IMO.
Read More »
Comment on GotW #3 Solution: Using the Standard Library (or, Temporaries Revisited) by Herb Sutter
@Ran: cppreference.com seems to have it correct: http://en.cppreference.com/w/cpp/language/range-for
Read More »
Comment on GotW #2 Solution: Temporary Objects by Herb Sutter
@Ahmed: I used to agree with you — the original GotW and Exceptional C++ said “hoist end()”. I switched to the current recommendation at Bjarne’s urging because compilers have for a long time routinely inlined the container.end() case in particular. Re (1), no you don’t need to inline both find_addr and end, just end — find_addr is its own function. Re (2), I agree one should write the clearest code first, so the extra variable for end is arguably less clear. But yes, range-for and std::find are the preferred answer anyway, see GotW #3.
Read More »
Comment on GotW #2 Solution: Temporary Objects by Ahmed Charles
I disagree with both points given which suggest that the end iterator being recomputed each loop should be preferred.
1) After seeing Chandler’s talk on Optimizations at C++Now, I’m significantly less likely to think that compilers will optimize this sort of code into ‘the right thing’. It would require inlining find_addr and end, while hoping that emps is a stack local in the caller.
2) Ignoring optimizations and compiler quality: For the same reasons that using algorithms is better than a normal for loop, because they are constrained and therefore provide more information to the reader, a for loop with the end calculated once provides more information to the reader, in that it guarantees that end isn’t changing (or if it is, that it’s likely to be a bug). I also don’t view this as a premature optimization, because it should simply be idiomatic to write code with the begin and end iterators assigned to variables in the for loop initializer, much like writing ++i is idiomatic. Granted, it should be idiomatic to write this as a range based for loop or std::foreach with a lambda, rather than a normal for loop. And with C++14, both of these are probably equivalent or less typing and they both cause end to be called only once.
Read More »
Comment on GotW #3 Solution: Using the Standard Library (or, Temporaries Revisited) by Ran
Herb, can you please elaborate on the underneath work that is done in ranged-based loops? As getting higher in the abstraction we also loose contact with the ground. And though we KNOW we are on the safe side and performance is taken into consideration, still, can you please explain more on what the standard says to implementers of range based loops?
Read More »
Comment on GotW #3 Solution: Using the Standard Library (or, Temporaries Revisited) by David Thomas
Since employee supports find(…, name), instead of e.name() == name the lambda could contain e == name. If name() returns a reference to const string, then I suspect it won’t matter. If name() returns a string, then I suspect we are paying price greater than e == name (on each iteration). Writing the lambda to avoid the potential temporaries yeidlds:
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; } ); return i != end(emps) ? i->addr : ""; }
On the compiler that I checked, if name() returns a const string&, then e == name optimizes to the same code as e.name() == name. No penalty here. However, if name() returns a string, then the impact is significant (the complexity prevented inlining and temporaries were created).
Read More »
Comment on GotW #3: Using the Standard Library (or, Temporaries Revisited) (3/10) by Róbert Dávid
@Olaf van der Spek: The other guys already told about find_if being self documenting: To discover that this code is doing a search, you only need to read the function name in the find_if case, but for range for, you need to check the loop body, and if the condition is more complex than this, you might need to take more than 0.1s to discover that it is a search. It’s also less error prone (no silently compiling mistakes e.g. if( name = emp.name() ) ).
And if you are into unit testing, you would also like that find_if version has only 2 branches (I’m not really into testing STL implementation), while the range-for version has 4 (depending on how you define branches), so you need more test cases for 100% code coverage.
Read More »
Comment on GotW #4: Class Mechanics (7/10) by litb1
You also need to consider multi element initializer lists when making a constructor “explicit” (see the tuple “explicit” constructor).
The question becomes, do you want this to work?
void f (complex c);
f ({ 0.f, 1.f });
Simply making the two parameter constructor explicit just to guard against the one-argument case then is unsuitable because it guards against the multi arg case too. You will need to overload it (possibly using constructor delegation in the implementation).
Read More »
Comment on GotW #4: Class Mechanics (7/10) by GregM
“Just focusing one issue I haven't seen mentioned by others already, I'd question the appropriateness of offering the increment operators at all.”
I had a similar thought when I read the class, “what exactly does it mean to increment a complex number?”. I don’t think I’ve ever used increment on a floating point number.
Read More »
Comment on GotW #4: Class Mechanics (7/10) by Klaim - Joel Lamotte (@MJKlaim)
To make an easy to use interface:
1. Make it impossible or very hard to use incorrectly (fail to compile, crashes, etc when used incorrectly – see std::chrono as a simple example) – this is the most important guideline for type design because it both prevent errors by mistake and, more importantly, it drives the usage (it’s like walls for a blind: when you touch it you know you can’t go this way);
2. Make sure each operation have only one way to be done – this actually improve composition (note that if two functions does the same thing but in a different way that impact the semantic, it’s two operations);
3. Hide complexity. Unfortunately it is not always efficient to hide totally the implementation details of our types (maybe it will be with modules but don’t count on it until we got implementations), but if you can hide the details, the only thing visible is how to use it.
4. Give a unique responsability to the type, not more.
5. Make it as short as possible, that is, a minimum of functions (depends on the domain and responsability);
6. Use names as obvious as possible in the domain context;
7. Document the interface. I put this at last because I consider it of last resort but you always end up doing it for member functions doing not that obvious operations.
Read More »
Comment on GotW #3 Solution: Using the Standard Library (or, Temporaries Revisited) by GregM
Dave, I asked the strlen question in GoTW #1, and Herb assures me that it’s not there is a “good” string library. It checks for the first character being 0 before it calls strlen.
While equality may feel simpler than inequality, I generally prefer to check for success rather than failure when the two options are basically equivalent.
Read More »
Comment on GotW #1 Solution: Variable Initialization – or Is It? by Seungbum Woo
I am using GCC 4.8.1 under Windows7 ( using MinGW-w64 ) I am using an experimental version of MinGW-w64 for its std::thread support. I should try MinGW-w64′s released version.
I confirmed that VC++ 2012 Nov CTP works fine with v{vv}.
Thank you for your answer.
Read More »
Comment on GotW #4: Class Mechanics (7/10) by Dave Harris
Interfaces are easy to use correctly if they follow established conventions and best practices. (Ideally your established conventions will reflect best practice, but best practice evolves, so the longer your conventions have been established the greater the danger they are out of date.) They should follow the principle of least surprise. Ideally a reasonably skilled programmer using them would not need to look up the documentation to see how they work.
The example class is presumably intended to be a numeric type, so it should follow the conventions established by int, double and other numeric types. It doesn’t. Note that its definition of operator+() isn’t [i]intrinsically[/i] wrong; it isn’t impossible to use correctly: but it is different to how operator+() works for double so very liable to be used incorrectly. It doesn’t matter if the documentation describes the idiosyncratic behaviour correctly and lucidly. Few programmers will think to read it.
Programmers have ample opportunities to be creative. Deciding the behaviour of operator+() is not one of them.
Read More »
Comment on GotW #4: Class Mechanics (7/10) by Mikhail Belyaev
Another thought: if we do not make it a template (which we possibly do not want) the whole class is actually better off being a constexpr class. It won’t hurt, it’ll help in some cases and it is certainly a good thing to do with classes this simple.
It won’t optimize anything, but lets you actually say in your code that that whole bunch of complex arithmetics on constants should run at compile-time.
Read More »
Comment on GotW #1 Solution: Variable Initialization – or Is It? by Herb Sutter
@Seungbum Woo: Looks like a compiler bug. What compiler and environment are you using?
I just tried:
- GCC 4.8 and VC++ 2012 Nov CTP which both like it;
- GCC 4.7.2 which thinks F f{a}; is illegal; and
- Intel ICC 13.0.1 which thinks v{vv} is illegal.
I believe GCC 4.8 and VC++ 2012 Nov CTP have it right.
Read More »
Comment on GotW #3 Solution: Using the Standard Library (or, Temporaries Revisited) by Dave Harris
Am I the only person who would prefer:
return i == end(emps) ? "" : i->addr;
Equality feels slightly simpler than inequality. I’d also prefer to avoid converting the c-string to a std::string:
return i == end(emps) ? string() : i->addr;
The conversion probably won’t allocate memory, but it’s still doing unnecessary work – possibly including a call to strlen(), for example.
Read More »
Comment on GotW #3 Solution: Using the Standard Library (or, Temporaries Revisited) by Herb Sutter
@Olaf: OK, clarified (in GotW #2) along the lines that "I'm assuming something like .name() is available and .name() == name has the same semantics."
@Sam: Good point, noted.
@Adrian: A program that does that has undefined behavior and could achieve the same effect by dereferencing a random int cast to a pointer. Within defined behavior, the range-for can iterate only one way, from the front in order visiting every element exactly once, and the only possibly variation in behavior is that it could stop early if there’s a break or return inside vs. visiting all elements.
Read More »
Comment on GotW #2 Solution: Temporary Objects by Herb Sutter
@Robert: That’s not the same example as in the article. :) I agree your two lines are equivalent, and in both cases the calling code can easily see the bug locally in code review just by knowing the return type of the function (by value). What’s different to me in find_addr is that we’d be returning a reference that refers, not “directly within this here object I’m returning,” but “deep within some data structure plus I’m not going to tell you exactly which node it is either.” The article was doing the latter, and there’s no way to check the calling code’s correctness during code review without carefully reading the English documentation of find_addr to figure out if the reference was still valid. Doing the former, returning a reference directly into this visible object the caller knows about, is much less problematic IMO and does not require English documentation analysis to spot the bug.
@tivrfoa: I just compared using “no optimizations” vs. “max optimizations” compiler flags.
Read More »
Comment on GotW #4: Class Mechanics (7/10) by mttpd
Good points in the previous comments.
One more thing which comes to mind — given that it makes sense for “complex” to have value semantics (as opposed to reference semantics), we’d probably want to consider preventing inheritance (for the same reason it’s not a good idea to inherit from an STL container).
As in:
Guideline #4: A base class destructor should be either public and virtual, or protected and nonvirtual.
http://www.gotw.ca/publications/mill18.htm
I don’t see the case for public & virtual (since I don’t see the case for our “complex” class having reference semantics). Hence, I’d go with the other alternative.
This is where C++11 comes in, as it allows us to use “final” to prevent inheritance:
http://en.cppreference.com/w/cpp/language/final
Read More »
Comment on GotW #4: Class Mechanics (7/10) by Adrian
Just focusing one issue I haven’t seen mentioned by others already, I’d question the appropriateness of offering the increment operators at all.
The increment and decrement operators make intuitive sense for discrete types, like counters, chars, certain enumerations, even bools.
Perhaps because I’ve always thought of these operators as analogues of Pascal’s succ() and pred(), it’s always seemed odd to me that C and C++ provide them for floating point types. What does it mean to increment a type that approximates a continuous quantity? It’s not intuitive (to me), given a double d, what
++d
would mean. Does it increment by 1.0 or does it increment by some epsilon to get to the next higher, representable value? Both would be useful, and, given that I could always write
d += 1.0;
for the former, defining operator++ for the latter would make the most sense. But alas, that’s not the case. That might just be my own bias.
Even if I make peace with increment and decrement on a double, what should they mean on a complex? That’s even less intuitive. A complex is a two-dimensional quantity. Should increment affect the real part or the imaginary part or both? It’s simply not obvious–at least not to everyone.
In the interest of making it easy to use correctly and difficult to use incorrectly, I’d leave out the increment and decrement operators for this class at least until a client of the class made a strong case for them. (A strong case might be that having these operators enables a class or function template that otherwise can’t use them.) In the mean time, a client can write
c += 1.0;
, which is not only correct, it’s _obviously_ correct. To everyone.
To me, that’s the difference between a well-implemented class interface and a well-_designed_ one.
Read More »
Comment on GotW #3 Solution: Using the Standard Library (or, Temporaries Revisited) by Adrian
“When you see B [the range-based for loop], you know for certain without inspecting the body of the loop that it is a loop that visits the element of emps in order.” Certainly there are some things the body _might_ do that would contradict that. For example, if the body modifies the container in a way that would invalidate outstanding iterators, you no longer can be sure that the loop will proceed normally. Consider:
for (const auto & e : emps) { if (e.name == "Herb") { emps.push_front(employee("Bjarne")); } }
Obviously, this isn’t an advisable thing to do in the loop, but the compiler isn’t likely to notice. Thus a range-based for loop might give you more confidence that it visits each element in order, but, unless emps is const, you cannot be absolutely certain without looking inside the body and without knowing the type of container and that container type’s iterator invalidation rules. Right?
Read More »
Comment on GotW #3 Solution: Using the Standard Library (or, Temporaries Revisited) by Sam Kramer
The astute reader will have noticed that the guidance from this tip is derived directly from the book Effective STL, Item 43: Prefer algorithm calls to hand-written loops. It’s good to know that best practices from C++98 are still largely relevant in C++11.
Read More »
Comment on GotW #2 Solution: Temporary Objects by tivrfoa
Hello. “with aggressive optimizations”. How are these “aggressive optimizations”? Could someone show an example? Thank you.
Read More »
Comment on GotW #4: Class Mechanics (7/10) by Rainer Blome
1. What makes interfaces "easy to use correctly, hard to use incorrectly"?
The first answer coming to my mind is “Documentation”.
The standard classes are quite well documented – imagine how difficult it would be to use them correctly without this documentation, and how easy it would be to use them incorrectly.
It makes me a little sad that no one mentioned this so far.
Among the very effective programmers that I got to know, the share of those who actually write good documentation is surprisingly low. There are even some who explicitly argue against documentation. I often wonder why this is so.
Reasons might be:
* Documentation is (usually) not being paid for. What usually matters is functionality.
* Not every one is a born writer. Some very effective programmers that I got to know write surprisingly bad natural language. My guess is that most of these actually know this (but some don’t :-). Many people dislike doing things that they are not good at. Thus I assume that some simply hate writing documentation because it makes them feel bad, or even inadequate.
* Writing good documentation is a skill that by most people must be learned. This takes at least effort and time, and thereby usually money.
* Tools that check for language errors in documentation are not widespread. This is in contrast to the code, where the compiler and tools like lint can help to work out the kinks.
Granted, in contexts where the code is not likely to ever be read, writing documentation has only one benefit that I can think of: The writer can hone his writing skills.
However, in contexts where the code is likely to be read, maintained or reused, documentation is crucial. In my professional experience (15y), this is the case in by far the majority of projects. In some circumstances, especially when key people leave a project, missing or bad documentation can become extremely expensive.
Read More »
Comment on GotW #4: Class Mechanics (7/10) by Marc
Just had to give it a final try.
class complex { // allow access to private members friend ostream& operator<<( ostream& os, const complex& c ); public: // doubles as default ctor complex( double r = 0, double i = 0 ) : real(r), imag(i) { } // to efficiently implement operator+ complex& operator+=(const complex& c) { real += c.real; imag += c.imag; return *this; } // return reference to result complex& operator++() { ++real; return *this; } // return unchanged object by value complex operator++( int ) { auto temp = *this; ++real; return temp; } // ... more functions that complement the above ... private: double real, imag; }; // IO operator cannot be a class member. // Class member ops require the left operand to be a class object ostream& operator<<( ostream& os, const complex& c ) { os << "(" << c.real << "," << c.imag << ")"; return os; } // allows implicit conversion of operands complex operator+ ( const complex &c1, const complex& c2 ) { complex c(c1); c += c2; return c; }
Read More »
Comment on GotW #3 Solution: Using the Standard Library (or, Temporaries Revisited) by Barry
This extra branch;
return i != end(emps) ? i->addr : "";
Find just returns an iterator that you still have to check. The range-for version just returns.
Read More »
Comment on GotW #2 Solution: Temporary Objects by Róbert Dávid
@GregM:
std::get<0>()
(blog ate my pointy braces) isn’t a member function of std::tuple, still isn’t returning by value..
@Herb: Wait a minute. Are you saying that
auto& x = function_that_creates_widget_and_returns_by_value().get_name();
is ‘good’, but
auto& x = get_widget_name(function_that_creates_widget_and_returns_by_value());
isn’t? I so far failed to create any example where a get(xx) construct can have a dangling reference what is fixed just by replacing that call to a xx.get() construct, that’s why I think the two are equivalent, thus should use the same guidelines for return type.
Read More »
Comment on GotW #4: Class Mechanics (7/10) by Brian M
Think I would as reviewer simply say use a std:: complex or other library and go off and have lunch!
But was never any good with made up exam questions….
ps Unfortunately saw similar when someone was writing a linked list class. It was a good lunch!
Read More »
Comment on GotW #3 Solution: Using the Standard Library (or, Temporaries Revisited) by Brian M
One important thing is that abstraction does not always equate to better or more readable code. The use of more familiar constructs is often a better overall solution, errors are more likely to be seen by the casual reader etc. Although the examples are fairly trivial some are certainly more comfortable and easier to read and Jay gets it about right with his comment. The important thing to remember is that code is just a means to an end! If we are thinking at this level of trivia is there something else wrong?
Read More »
Comment on GotW #3 Solution: Using the Standard Library (or, Temporaries Revisited) by Olaf van der Spek
@Herb Sure. IMO you disregarded this note: “Note: As with GotW #2, don't change the semantics of the function, even though they could be improved.”
Whether employee == string is valid and has the same semantics as employee::name() == string isn’t given, so you shouldn’t assume it is.
Read More »
Comment on GotW #4: Class Mechanics (7/10) by khurshid
Sorry, forgot default ctor.
class complex{ complex() = default; complex( const complex& other) = default; complex& operator = (const complex& other) = default; // other method and ctors .............................. }
Read More »
Comment on GotW #4: Class Mechanics (7/10) by khurshid
class complex { public: constexpr explicit complex( double r, double i = 0.0 ) noexcept : real_ { r } , imag_ { i } { } constexpr complex operator+ ( const complex& other ) const noexcept { return complex( real_ + other.real_, imag_ + other.imag_ ); } complex& operator++() noexcept { ++real_; return *this; } complex operator++( int ) noexcept { auto temp = *this; ++real_; return temp; } // ... more functions that complement the above ... constexpr double real()const noexcept { return real_; } constexpr double imaginary() const noexcept { return imag_;} private: double real_ = 0.0; double imag_ = 0.0; }; std::ostream& operator << (std::ostream& os, const complex& c) { return os << '(' << c.real() << ',' << c.imaginary() << ')' ; }
Read More »
Comment on GotW #4: Class Mechanics (7/10) by Marc
But made error(s). So, back to work
Read More »
Comment on GotW #4: Class Mechanics (7/10) by Marc
Thought I’d give it a try
using namespace std; // IO operator cannot be a class member. // Class member ops require the left operand to be a class object ostream& operator<<( ostream& os, const complex& c ) { os << "(" << c.real << "," << c.imag << ")"; return os; } complex operator+ ( const& complex c1, const& complex c2 ) { complex c = c1; c.real += c2.real; // '+=' assumed c.imag += c2.imag; return c; } class complex { // allow access to private members friend ostream& operator<<( ostream& os, const complex& c ); friend complex operator+ ( const& complex c1, const& complex c2 ); public: complex( double r = 0, double i = 0 ) : real(r), imag(i) { } // doubles as default ctor complex& operator++() // return reference to result { ++real; return *this; } complex operator++( int ) { auto temp = *this; ++real; return temp; } // ... more functions that complement the above ... private: double real, imag; };
Read More »
Comment on GotW #4: Class Mechanics (7/10) by Eric Duhon (@duhonedd)
I figured using user literals would helps with syntax and avoid implicit cast problems from float(i.e. was the float a real or imaginary). Ended up with a lot of code though.
#include
using namespace std;
//template
class Imaginary
{
public:
constexpr Imaginary() = default;
constexpr Imaginary(const Imaginary&) = default;
Imaginary& operator=(const Imaginary&) = default;
explicit constexpr Imaginary(double _value) : m_Value(_value) { }
Imaginary& operator-()
{
m_Value = -m_Value;
return *this;
}
Imaginary& operator+=(const Imaginary& _other)
{
m_Value += _other.m_Value;
return *this;
}
ostream& PrintToStream(ostream& _stream) const
{
_stream << m_Value << 'i';
return _stream;
}
private:
//Don't want getters/setters for this, don't want a way to treat this as a standard float.
double m_Value = 0;
};
Imaginary operator+(const Imaginary& _arg1, const Imaginary& _arg2)
{
Imaginary result = _arg1;
result += _arg2;
return result;
}
ostream& operator<<(ostream& _stream, const Imaginary& _value)
{
return _value.PrintToStream(_stream);
}
//could use _fi or something for Imaginary and then templatize the Complex and Imaginary class
constexpr Imaginary operator"" _i (long double _value)
{
return Imaginary(static_cast(_value));
}
constexpr Imaginary operator"" _i (unsigned long long _value)
{
return Imaginary(static_cast(_value));
}
class Complex
{
public:
constexpr Complex() = default;
constexpr Complex(const Complex&) = default;
Complex& operator=(const Complex&) = default;
constexpr Complex(double _real, const Imaginary& _imag) : m_Real(_real), m_Imag(_imag) {}
constexpr Complex(double _real) : Complex(_real, 0_i) {}
constexpr Complex(const Imaginary& _imag) : Complex(0, _imag) {}
Complex& operator+=(const Complex& _other)
{
m_Real += _other.m_Real;
m_Imag += _other.m_Imag;
return *this;
}
Complex& operator++()
{
*this += 1;
return *this;
}
Complex operator++(int)
{
Complex result = *this;
++(*this);
return result;
}
ostream& PrintToStream(ostream& _stream) const
{
_stream << m_Real << "+" << m_Imag;
return _stream;
}
private:
double m_Real = 0;
Imaginary m_Imag = 0_i;
};
Complex operator+(const Complex& _arg1, const Complex& _arg2)
{
Complex result = _arg1;
result += _arg2;
return result;
}
Complex operator+(double _real, const Imaginary& _imag)
{
return Complex(_real, _imag);
}
Complex operator+(const Imaginary& _imag, double _real)
{
return Complex(_real, _imag);
}
ostream& operator<<(ostream& _stream, const Complex& _value)
{
return _value.PrintToStream(_stream);
}
int main(int argc, const char * argv[])
{
auto im = -4_i;
cout << im << endl;
auto im2 = 2_i + im;
cout << im2 << endl;
Complex cx1 = 1 + 3_i;
cout << cx1 << endl;
Complex cx2 = 4_i + -3;
cx2 += cx1;
cout << cx2 << endl;
cout << ++cx2 << endl;
cout << cx2++ << endl;
cout << cx2 << endl;
cx2 = cx2 + cx1;
cout << cx2 << endl;
cx2 += -14_i;
cout << cx2 << endl;
return 0;
}
Read More »
Comment on GotW #3 Solution: Using the Standard Library (or, Temporaries Revisited) by Lars Viklund
Aren’t generic lambdas C++1y?
Read More »
Comment on GotW #4: Class Mechanics (7/10) by Alf P. Steinbach
The notion of “easy to use correctly and hard to use incorrectly” is generally a good idea, but not always. As an example, a simple 2D mutable Point class is easiest to use correctly when it just has two public members and a constructor or two. Improbable as it may seem, for double>/code> members Visual C++ has a hard time optimizing optimizing code that accesses the members indirectly, or at least, it still was a bit deficient re such optimizations the year before last, so this has also to do with fundamental low-level efficiency.
The "easy to use correctly" has to do with the class as a provider of practically useful operations and with the class as a single well understood abstraction, which is more at the language independent design level, while the "hard to use incorrectly" has to do with the class as an enforcer of design constraints, which is more at the language specific and even compiler specific level, Accommodating Visual C++'s optimization problems is an example of possibly valid compiler-specific concerns. I think such choices need to be DOCUMENTED in order to be understood by maintenance programmers or even just by client code programmers.
Having noted that efficiency is a valid consideration, one should also note that it's easy to walk into the premature optimization trap. And the class presented in this question looks like an example of that. Apparently it's meant to provide a convenient increment operator, a
operator++, but that would be better provided for a class derived from std::complex<>T (derived class in order to minimize the problem of multiple definitions of such an operator).
The above addresses some main concerns of the "JG" question, question 1. Others have already discussed the trivia of the "Guru" question, question 2, what the compiler can tell you if you give it the code. I think the "Guru" designation for that is a bit misplaced: nowadays it's not hard to feed some code to a compiler.
So far the existing answers have failed to mention what a compiler can't tell you, such as the pros and cons of choosing `void` as result type for `operator++`. Much in the same vein as choosing an as yet somewhat unconventional signed size type. This has to do with the more challenging "hard to use incorrectly" aspect – challenging in part because some established conventions in C++ are at odds with that goal.
Read More »
Comment on GotW #3 Solution: Using the Standard Library (or, Temporaries Revisited) by Balakrishnan B
I dont understand what does an extra branch mean in the find_if version. Does everyone refer to a function call? I guess algorithms like find_if would be most likely be inlined as it takes template arguments.
Read More »
Comment on GotW #4: Class Mechanics (7/10) by infogulch
GregM “It is when all but one of the parameters is defaulted.”
Ah I didn’t catch that, thanks.
Read More »
Comment on GotW #4: Class Mechanics (7/10) by Crazy Eddie
Well, others have mentioned that the postfix ++ should be based on prefix ++ so I wont reiterate that. The problem with << being part of the class likewise has been covered, but making it a friend is a mistake.
What this class needs is 'getters' for real and imag. Then + and << can be implemented as non-friend functions. Although setters for simple data in a class is sometimes a mistake, it's warranted here. The sanity that the operators provide is simply to keep two values locked together in a sensible way. There are a great many reasons why we'd want getters for real and imag and the class is rather useless without them.
Adding the getters (not necessarily setters) also obeys the principle that you should be able to tell if two objects are equal based on their public interface alone. In other words, we want to be able to write == as a non-friend function as well. I forget where I got this principle but it's a good one.
That said, operator + should defer to +=, which should be added.
complex operator + (complex left, complex const& right) { return left += right; }
The compiler will get rid of the unnecessary temporaries either by using move construction or RVO.
Read More »
Comment on GotW #2 Solution: Temporary Objects by Rick Yorgason
@Herb But in practice, I’ve never seen it cause much of a problem. Any time a function is returning a &, whether it’s a getter or a function like yours, the caller always knows one thing: the return value is valid if used immediately.
So if you’re returning a const&, I know that both of these are safe:
A) string s = find_addr(emps, “Rick”);
B) if(find_addr( emps, “Rick” ).find( “Toronto” ) != string::npos) {}
But if you’re returning a value, one of those is less efficient.
I agree that hanging references/iterators is a very common problem, but every time I’ve encountered it, the caller made a conscious decision to store a reference rather than a value, including in the example you provided.
Read More »
Comment on GotW #4: Class Mechanics (7/10) by Mikhail Belyaev
Follow-up:
My bad. It is generally better to write operator+ as a friend for classes with implicit constructors:
//... inside the class friend complex operator+(const complex& lhv, const complex& rhv) { return { lhv.real+rhv.real, lhv.imag+rhv.imag }; } //...
This way both cmplx + 2.2 and 2.2 + cmplx will work.
Read More »
Comment on GotW #4: Class Mechanics (7/10) by Mikhail Belyaev
// 1. why not use std::complex? class complex { public: // 2. where's the default/move/copy constructor? complex() = default; complex(const complex&) = default; // the 0 instead of 0.0 is not really an issue, but why not be a kind Samaritan? complex( double r, double i = 0.0 ): real(r), imag(i) { } // 3. this was messed up complex operator+ (const complex& other ) { return { real+other.real, imag+other.imag }; } // 4. this was way totally messed up friend ostream& operator << (ostream& os, const complex& cm) { return os << "(" << cm.real << "," << cm.imag << ")"; } // 5. as was this const complex& operator++() { ++real; return *this; } // 6. the postfix ++ was actually the only operator that worked as it should // but it's generally better to write it using prefix ++ complex operator++( int ) { auto temp = *this; ++*this; return temp; } // ... more functions that complement the above ... private: double real, imag; };
Read More »
Comment on GotW #4: Class Mechanics (7/10) by GregM
Joe “I don't think explicit is necessary because implicit conversion isn't possible with multi-parameter constructors.”
It is when all but one of the parameters is defaulted.
Read More »
Comment on GotW #4: Class Mechanics (7/10) by Tom
template<typename FloatType = double> class complex { public: inline complex(const FloatType r = FloatType(),const FloatType i = FloatType() ) : real{r}, imag{i} { } complex& operator+= (const FloatType& r) { real += r; return *this; } friend inline complex operator+ (const complex& a, const complex& b) { return {a.real + b.real, a.imag + b.imag}; } friend inline ostream& operator<<( ostream& os, const complex& cp ) const { return os << "(" << cp.real << "," << cp.imag << ")"; } inline complex& operator++() { ++real; return *this; } inline complex operator++( FloatType ) { auto temp = *this; ++real; return temp; } // ... more functions that complement the above ... private: FloatType real, imag; };
Read More »
Comment on GotW #3 Solution: Using the Standard Library (or, Temporaries Revisited) by Barry
Yes, in this specific example the fact that we’re returning a string by-value dwarfs the extra branch after the find(), but I am more interested in the general case, where we do call some function lots of times, and its return is very light (say it returns a pointer to the found element or 0).
Read More »
Comment on GotW #4: Class Mechanics (7/10) by Roger
I’m going to address one tiny part of this – the streaming. I’ve always hated this form of implementing printing. I don’t mean the fact that it is a member vs non-member function. I mean the assumption that one form of printing is good or correct. Perhaps it is due to the fact that I do a lot of map applications, but why the assumption that I have only one way to ‘print’ (stream) something? In my application domain, a latitude can be printed as decimal degrees (32.3443534N), as deg/min, as deg/min/sec, we can use N/S or +/- to denote hemispheres, and so on. I may want to stream it to a file in a different format, to a SQL database in yet another format, and so on.
Also, you may need to stream to the cout stream, you may need to use Windows’ WriteFile function, or who knows what sink.
So, first of all, any kind of printing or streaming does not belong in the class. And, it should not assume some single representation. Hey, if your app requires a single representation,sure, go for it, but I would think that a class like Complex would span across more than one app if you are working in a space where you need complex numbers.
And, of course, this considers only half of the issue. Why are we implementing streaming to output, but not streaming from input to the class? Perhaps that makes the wide variety of possible formats clearer – you may be reading a file generated from a 3rd party app, you might be reading user input with errors in it, and so on. Clearly there is no ‘one size fits all’ solution here. Yet we so often treat output that way.
In short, I almost never use the <> operators to define streaming for a class because it is something that reasonably has to vary (to be fair, I’ve always worked in industries where a piece of code like this is likely to hang around and be used for at least a few decades – I’d write a stream function like this in a second for ‘throwaway’ code)
Read More »
Comment on GotW #3 Solution: Using the Standard Library (or, Temporaries Revisited) by Herb Sutter
@Leo: This is an interesting observation, especially if instead of deducing const auto& we deduce the constraint more generally. I’ve started a thread based on your example at concepts@isocpp.org here: https://groups.google.com/a/isocpp.org/d/msg/concepts/mUlJ3D1VGVE/41JYIKu_004J . I don’t know if it’ll go anywhere, but it could address the concerns about the nice “[](e){ use(e); }” syntax too…
Read More »
Comment on GotW #4 Solution: Class Mechanics by Marshall
>> 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.
Read More »
Comment on GotW #4 Solution: Class Mechanics by Olaf van der Spek
complex operator+( const complex& lhs, const complex& rhs ) {
What about this?
complex operator+( complex lhs, const complex& rhs ) { lhs += rhs; return lhs; }
Read More »
Comment on GotW #4 Solution: Class Mechanics by jlehrer
In #5 I think you meant “the other reason to prefer non-members” not “the other reason to prefer members”.
Read More »
No comments:
Post a Comment