# Stupid C++ Question

This topic is 3001 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

## Recommended Posts

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.

##### Share on other sites
You have to make operator* a free (global) function, which takes two arguments (the left and right sides of the *). e.g.
XYZ operator*(float lhs, const XYZ& rhs) { return rhs * lhs; }
Also, your existing operator* should be a const member function:
XYZ XYZ::operator*(float s) const { ... }

##### Share on other sites
Quote:
 Original post by SamboriI 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;}

##### Share on other sites
Thanks. Works great!

##### Share on other sites
Quote:
 Original post by Washusnip, 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 :)

##### Share on other sites
Quote:
Original post by Grafalgar
Quote:
 Original post by Washusnip, 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.

##### Share on other sites
Quote:
 Original post by WashuYou 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) called

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

##### Share on other sites
Quote:
 Original post by Grafalgarcalls 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.

##### Share on other sites
*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).

##### Share on other sites
Quote:
Original post by Antheus
Quote:
 Original post by Grafalgarcalls 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.

1. 1
Rutin
19
2. 2
3. 3
JoeJ
16
4. 4
5. 5

• 26
• 20
• 13
• 13
• 17
• ### Forum Statistics

• Total Topics
631700
• Total Posts
3001781
×