Jump to content
  • Advertisement
Sign in to follow this  
Sambori

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.

If you intended to correct an error in the post then please contact us.

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 this post


Link to post
Share on other sites
Advertisement
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 this post


Link to post
Share on other sites
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;
}

Share this post


Link to post
Share on other sites
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 :)

Share this post


Link to post
Share on other sites
Quote:
Original post by Grafalgar
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.

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 this post


Link to post
Share on other sites
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 1

class 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_MUL
Foo operator*(float s, const Foo& foo)
{
#if USE_TEMP
Foo tmp = foo;
return tmp * s;
#else
return foo * s;
#endif
}
#else
Foo 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;
}
#endif

int _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 0

Output:

Foo() called

a * 1.0f:
Foo(copy) called

1.0f * a:
Foo(copy) called




#define USE_TEMP 1
#define USE_ASSIGN_MUL 0

Output:

Foo() called

a * 1.0f:
Foo(copy) called

1.0f * a:
Foo(copy) called
Foo(copy) called




#define USE_TEMP 1
#define USE_ASSIGN_MUL 1

Foo() called

a * 1.0f:
Foo(copy) called
Foo(copy) called

1.0f * a:
Foo(copy) called
Foo(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 this post


Link to post
Share on other sites
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.

Share this post


Link to post
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 this post


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


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.

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!