Stupid C++ Question
Author
I define a class called XYZ that has an overloaded operator * that takes in a scalar as argument.
XYZ XYZ::operator*(float s) { ... }
this works for:
XYZ xyz1, xyz2;
xyz1 = xyz2 * s;
How can I make it work for:
xyz1 = s * xyz2
???
What I've learned so far that I cannot. Correct me if I'm wrong.
Thanks.
You have to make operator* a free (global) function, which takes two arguments (the left and right sides of the *). e.g.
XYZ XYZ::operator*(float s) const { ... }
XYZ operator*(float lhs, const XYZ& rhs) { return rhs * lhs; }XYZ XYZ::operator*(float s) const { ... }
Quote:Original post by Sambori
I define a class called XYZ that has an overloaded operator * that takes in a scalar as argument.
XYZ XYZ::operator*(float s) { ... }
this works for:
XYZ xyz1, xyz2;
xyz1 = xyz2 * s;
How can I make it work for:
xyz1 = s * xyz2
???
What I've learned so far that I cannot. Correct me if I'm wrong.
Thanks.
XYZ operator*(float s, XYZ const& b) { XYZ tmp = b; return tmp * s;}Ideally though you shouldn't define operator* as a member function at all, but instead operator *=, then you can simply rewrite both functions as:
XYZ operator*(float s, XYZ const& b) { XYZ tmp = b; return tmp *= s;}XYZ operator*(XYZ const& a, float s) { XYZ tmp = a; return tmp *= s;}Quote:Original post by Washu
snip, snip
Your implementations are a bit inefficient. In the first one, you're already provided a const XYZ, no reason to make a temporary and operate on that. We can make XYZ::operator* a const function, and in operator*(float, const XYZ&) use the const object as-is.
Similarly, if performance is a concern you shouldn't implement both versions using operator*=, since they both create another temporary just to use operator*=.
Explicitly writing operator* functions to avoid unnecessary constructor calls are the way to go here :)
Quote:Original post by GrafalgarQuote:Original post by Washu
snip, snip
Your implementations are a bit inefficient. In the first one, you're already provided a const XYZ, no reason to make a temporary and operate on that. We can make XYZ::operator* a const function, and in operator*(float, const XYZ&) use the const object as-is.
You might think they are, but they aren't. The compiler has the ability to elide construction of temporary objects, and even deal with NRVO (any modern compiler can do this). As such premature optimizations like code duplications are not only unnecessary, but frequently can introduce bugs that require changing multiple identical pieces of code to fix.
Quote:Original post by Washu
You might think they are, but they aren't. The compiler has the ability to elide construction of temporary objects, and even deal with NRVO (any modern compiler can do this). As such premature optimizations like code duplications are not only unnecessary, but frequently can introduce bugs that require changing multiple identical pieces of code to fix.
I quickly ran a couple of tests:
#include "stdafx.h"#define USE_TEMP 1#define USE_ASSIGN_MUL 1class Foo{public: Foo() { printf("Foo() called\n"); } Foo(const Foo& src) { printf("Foo(copy) called\n"); }#if !USE_ASSIGN_MUL Foo operator*(float s) const { return *this; }#endif Foo& operator*=(float s) { return *this; }};#if !USE_ASSIGN_MULFoo operator*(float s, const Foo& foo){#if USE_TEMP Foo tmp = foo; return tmp * s;#else return foo * s;#endif}#elseFoo operator*(float s, const Foo& foo){ Foo tmp = foo; return tmp *= s;}Foo operator*(const Foo& foo, float s){ Foo tmp = foo; return tmp *= s;}#endifint _tmain(int argc, _TCHAR* argv[]){ Foo a; printf("\na * 1.0f:\n"); a * 1.0f; printf("\n1.0f * a:\n"); 1.0f * a; return 0;}Visual Studio, 2008, default Win32 Console program, built for Release.
Here's my output, with the defines set:
#define USE_TEMP 0#define USE_ASSIGN_MUL 0Output:Foo() calleda * 1.0f:Foo(copy) called1.0f * a:Foo(copy) called#define USE_TEMP 1#define USE_ASSIGN_MUL 0Output:Foo() calleda * 1.0f:Foo(copy) called1.0f * a:Foo(copy) calledFoo(copy) called#define USE_TEMP 1#define USE_ASSIGN_MUL 1Foo() calleda * 1.0f:Foo(copy) calledFoo(copy) called1.0f * a:Foo(copy) calledFoo(copy) calledYour implementation, using *=, calls the copy constructor twice, one for each of the temporary objects it needs to use operator *= on. This makes sense to me, I do not see how the compiler can optimize out something that modifies an object, as is the case with operator*=.
Even having a temporary in operator*(float, const Foo&) resulted in an extra copy constructor call. Now that temporary could arguably be optimized out, but it gets tricky when you deal with heavy objects with potential side effects.
Quote:Original post by Grafalgar
calls the copy constructor twice
printf is not side-effect free, so compiler is not allowed to optimize it out.
Remove printf from constructors and examine assembly.
*sigh* just because the copy constructor can be elided doesn't mean that it will be. If the copy constructor is non-trivial (as in if you perform a complex operation such as a function call that cannot be trivially inlined), then the compiler will not be able to elide the copy construction.
Perhaps you should try a more realistic example, such as that of a vector (which is what one would assume he's attempting to implement).
Perhaps you should try a more realistic example, such as that of a vector (which is what one would assume he's attempting to implement).
Quote:Original post by AntheusQuote:Original post by Grafalgar
calls the copy constructor twice
printf is not side-effect free, so compiler is not allowed to optimize it out.
Remove printf from constructors and examine assembly.
Ah, and therein lies my point. We don't know the specifics of the XYZ object that the OP referenced, but if it's a heavy object with side-effects when the copy-constructor is called, then having those temporaries can be quite troublesome.
This is not a premature optimization, this is knowing the consequences of code and the subtleties of C++. I understand the need to have generic, re-usable code, but in this case I consider that over-engineering at the potential cost of performance.
Quote:Original post by Washu
*sigh* just because the copy constructor can be elided doesn't mean that it will be. If the copy constructor is non-trivial (as in if you perform a complex operation such as a function call that cannot be trivially inlined), then the compiler will not be able to elide the copy construction.
Perhaps you should try a more realistic example, such as that of a vector (which is what one would assume he's attempting to implement).
No need to get snippy. We're having a discussion, are we not? Anyway, that's exactly right: "does not mean that it will be." Your suggestion went to someone that seems new to the language, and you provided him with an implementation that could very well have a non-trivial performance impact. If he is, in fact, implementing a vector then that scalar mul could be called hundreds, if not thousands, of times a frame, and could produce a significant hit on the frametime.
As I said already, your solution, while software-engineering friendly, is an over-engineered one that could come with a non-trivial performance impact.
This topic is closed to new replies.
Advertisement
Popular Topics
Advertisement
