• Advertisement
Sign in to follow this  

is there any better way of writing operators in c++?

This topic is 4251 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

http://rafb.net/paste/results/gWylDH99.html some one told me that it is wrong to write it like that and that there is a better way..?

Share this post


Link to post
Share on other sites
Advertisement
You might take a look at this; it addresses operator scope (which I'm guessing is what the person was referring to), along with some other operator-related topics.

Share this post


Link to post
Share on other sites
1) Assignment style operators typically return a reference.
2) Having operator+ use operator+= can often save on logic duplication for anything nontrivial - e.g.:

friend MyInt operator+( const MyInt & lhs , const MyInt & rhs ) {  MyInt temp( lhs );  temp += rhs;  return temp; }


3) One typically wants to use the above instead of the operator+ with the implicit this, since this will allow: 5 + MyInt( 42 )

Share this post


Link to post
Share on other sites
Quote:
Original post by MaulingMonkey
2) Having operator+ use operator+= can often save on logic duplication for anything nontrivial - e.g.:


I like doing it the other way around: having operator+= use operator+. It just makes more sense because operator+ is more general and operator+= is more specific.

Share this post


Link to post
Share on other sites
Doing it that way removes the cheif advantage of using operator += in the first place. operator+ requires the creation of a temporary; operator += does not.

Share this post


Link to post
Share on other sites
Quote:
Original post by Deyja
Doing it that way removes the cheif advantage of using operator += in the first place. operator+ requires the creation of a temporary; operator += does not.


I never thought about it that way. I kind of trust the compiler to optimize the obvious. :-)

Share this post


Link to post
Share on other sites
Guest Anonymous Poster
the compile wont rewrite your functions to avoid the temporary in this case...

Share this post


Link to post
Share on other sites
Quote:
Original post by deathkrush
Quote:
Original post by Deyja
Doing it that way removes the cheif advantage of using operator += in the first place. operator+ requires the creation of a temporary; operator += does not.


I never thought about it that way. I kind of trust the compiler to optimize the obvious. :-)


Which is fine when it's obvious, but in more complex scenarios (especially if the code depends on other APIs) it becomes far less so. Given that it's mainly a cosmetic difference in terms of programming, it's a good habit to use the one that will "allways" work best - just like it ++i instead of i++ used to be recommended (and still probably will be).

Share this post


Link to post
Share on other sites
Quote:
Original post by clearly
http://rafb.net/paste/results/gWylDH99.html
some one told me that it is wrong to write it like that
and that there is a better way..?
If your are referring to the ++ operators from that link, the post increment is incorrectly imlemented. It should return copy of the old object, not a reference to the incremented object.

Share this post


Link to post
Share on other sites
I would suggest making MyInt( int ); different. Right now it can be implicitly casted from any int to MyInt, which can lead to confusion and troubles. Instead write it:
explicit MyInt( int );

Also I agree that += should be implemented in terms of +. You lose the benefit of no temporary, that is true. But you gain exception safety.
Case 1: If += begins modifying itself and an exception occurs half way through, the object is no longer correct and you are not exception safe.
Case 2: If += calls + and the exception occurs in +, the object's state hasn't been altered and you can propogate the exception.

Share this post


Link to post
Share on other sites
If your operator has a chance of leaving your object in an invalid state, you have a problem, and simply not using it in this one case isn't a very good solution.

Besides, consider why it might throw an exception. Simple addition is unlikely to throw an exception, so presumably it dies for memory related reasons. If this is the case, your assignment operator might throw an exception half way through while copying, and your problem still exists, you haven't fixed it!

I think + should be implemented in terms of +=, return Foo(*this)+=whatever;.

Share this post


Link to post
Share on other sites
Exception Safe Case:
MyInt operator +( const MyInt lhs, const MyInt rhs )
{
MyInt temp( lhs );
temp.m_Value += rhs.m_Value;
return temp;
}

MyInt & operator +=( const MyInt rhs )
{
MyInt temp( m_Value + rhs.m_Value );
Swap( temp );
return * this;
}

void MyInt::Swap( MyInt & rhs )
{
std::swap( m_Value, rhs.m_Value );
}

In this situation you will only run out of memory in the allocation of the temps. And if you ever run out of memory, your origional object is still in a valid state. So contrary to what you said, there isn't a problem. The problem is fixed. That's the whole point of exception safety.

Also you could use Swap( temp ); in many other places, like operator =. This helps you work on a seperate, temporary copy, preserving your origional state. You can read more about it in Exceptional C++.

Share this post


Link to post
Share on other sites
Also it is perfectly common for non-exception safe operators to leave an object in an invalid state. Something so simple as MyInt won't have a problem since its only operations are atomic. But take a class that is more complex:

Matrix & Matrix::operator *=( const Matrix & rhs )
{
// This is not atomic.
// If an error occurs half way through, our object is out of a valid state.
}

Same with vectors. Same with all of the std container library.

Suppose you are calling operator = on a container, which is templated over type T. You have no garuntee that T's operator = won't throw an exception.

And this is good because maybe T is something like database connections, where you may want only 4 max. Copying and trying to create 8 would be bad. T's operator = should throw in this case.

But that means your container's operator = can throw an exception half way through. If your origional had 3 connections, the first connection will copy without throwing. But then the second connection will throw. And now your container is in an invalid state.

Share this post


Link to post
Share on other sites
Perhaps our idea of an 'invalid state' differs, because the container will still contain all the objects upto the one that threw the exception, and be just as usable as any other container.

Share this post


Link to post
Share on other sites
I don't think it will be just as usable. First off a half-filled container when you expected a full container is already wrong. Second the copy will likely look like this:

m_Count = rhs.m_Count;
for( unsigned int i( 0 ); i < m_Count; ++i )
{
// copy item i
}

If this fails half way through, your count still shows an incorrect number. This is why NONE of the standard containers do this. It isn't exception safe. It doesn't "still work."

Share this post


Link to post
Share on other sites
You're saying that if operator += throws an exception, the object will be in an invalid state, and hence using the temporary fixes this.

I'm saying it doesn't because if operator += can can throw an exception, then operator = can also throw an exception, and that x=x+y could be just as wrong x+=y.

If operator = is sane enough not to leave an object in an invalid state, then there is absolutely no reason why operator += can't be sane enough to do the same. If you're going to have operator += to throw an exception, there's no reason why you can't do it before you've changed anything, or if you do change something, keep your object in an internally consistant state.

Share this post


Link to post
Share on other sites
Quote:
Original post by clearly
can any one rewrite my code and show me my mistakes?


Here ya go:


// NOOOOOOO!!!!!! <iostream.h> is NOT part of the C++ standard
// and provides DIFFERENT TYPES to that specified by the standard
// (and on my copy of VS2005 doesn't even exist). Standard C++
// headers drop the .h off the end of the filename.
#include <iostream>


// Don't be afraid of writing small functions inlined within the class.
class MyInt
{
public:

MyInt() : value(0)
{
}

// Use explicit if you don't want the compiler to automatically convert
// from an int to a MyInt. Not strictly necessary here and perhaps not
// even desired, but it can become an issue if you have multiple implicit
// casts that would allow the compiler to find strange ways of casting
// from one type to another that you're not expecting.
explicit MyInt(int x) : value(x)
{
}

// If your getter is doing nothing but return an exact copy
// of an internal variable, return a const reference as it
// reduces the number of temporaries created. Of course, with
// a plain old int this won't make any difference whatsoever.
const int& GetValue() const
{
return value;
}

// Assignment operators should always return a reference to the
// object being assigned, not a copy of it. Some people prefer
// to return a const reference to prevent this: (a = b) = c, but
// this is a personnal choice and the behaviour should be clearly
// stated in any documentation as it differs from the norm.
MyInt& operator=(const MyInt& rhs)
{
// Pointless optimisation!! It'll take longer to get the
// value and do the comparison than it would to just assign it.
//if(value==rhs.GetValue())
// return *this;

// If your doing something a bit more interesting and *really*
// need to do this sort of check, it'd be better to do it like this:
//int rhsValue = rhs.GetValue(); // Cache the result locally
//if(value!=rhsValue) value = rhsValue;

// Calling GetValue() isn't really necessary in this case, you
// can access rhs.value directly. It doesn't hurt though and can
// be a good thing if plan on manipulating the value in some
// way before using it (eg. clipping to a range of values).
value = rhs.GetValue();

return *this;
}

MyInt& operator+=(const MyInt& rhs)
{
value += rhs.GetValue();
return *this;
}

MyInt& operator-=(const MyInt& rhs)
{
value += rhs.GetValue();
return *this;
}

// Pre-increment: Increment the value first and then return self
MyInt& MyInt::operator++()
{
++value;
return *this;
}

// Post-increment: Make a copy of the value first, increment self
// and then return the COPY.
MyInt MyInt::operator++(int a)
{
MyInt old(*this);
++value;
return old;
}

private:

int value;

};


// operator+() uses 2 objects to create a third, and
// therefore doesn't belong to any single object.
// Hence it's kept external to the class.
MyInt operator+(const MyInt &lhs, const MyInt &rhs)
{
// Code reuse is fun!!
return MyInt(lhs) += rhs;
}


// Another good reason to keep stuff like operator+()
// outside of the class. You wouldn't be able to do
// this otherwise (ie. not having MyInt on the LHS)
MyInt operator+(int lhs, const MyInt &rhs)
{
return MyInt(lhs) += rhs;
}


// If we provide both this and the above overload
// of operator+() you can keep the cast constructor
// above as explicit (which is a good thing)
MyInt operator+(const MyInt &lhs, int rhs)
{
return MyInt(lhs) += MyInt(rhs);
}


// Play nice with iostreams!
std::ostream& operator<<(std::ostream& out, const MyInt& rhs)
{
return out << rhs.GetValue();
}



int main()
{
// There's no such thing as cout or endl, this is part of why you
// shouldn't use <iostream.h>. It's std::cout and std::endl.
// If you want the short-hand versions use the 'using' keyword.
MyInt a(10000);
std::cout << a.GetValue() << std::endl;

// No need for a.GetValue() if we provide an overload
// of operator<<() that works with iostreams.
MyInt b = a + 100;
std::cout << b << std::endl;

MyInt c = a + b;
std::cout << c << std::endl;

a += c;
std::cout << a << std::endl;

a -= c;
std::cout << a << std::endl;

return 0;
}


Share this post


Link to post
Share on other sites
Quote:
Original post by joanusdmentia
Quote:
Original post by clearly
can any one rewrite my code and show me my mistakes?


Here ya go:

*** Source Snippet Removed ***


thank you very much..
you learned me alot, but i got few things that im not totally understand
like:

explicit MyInt(int x) : value(x)
{
}



what does the explicit do, and how does it help me?

and when does this happan:

MyInt operator+(const MyInt &lhs, const MyInt &rhs)
{
// Code reuse is fun!!
return MyInt(lhs) += rhs;
}



and about the others..
MyInt& operator+=(const MyInt& rhs)

cant i just write "MyInt" instade of "MyInt&" ?
what is the diffrent?
and why does the operator+ has to be out of the "class members"?

Share this post


Link to post
Share on other sites
Quote:
Original post by clearly
what does the explicit do, and how does it help me?


When you have a constructor of the form MyType::MyType(OtherType val) the compiler will use it to perform a cast from OtherType to MyType. The default behaviour of the compiler is to implicitly perform this cast whenever it sees the need to (eg. when passing an object of OtherType to a function that takes a MyType), but by specifying the explicit keyword you're telling the compiler "only use this constructor when I explicitly ask for it".

Implicit casts can cause problems because the compiler will use as many casts as necessary to try and get from one type to another, so if it can find a series of 10 different implicit casts to get from type A to type B it'll use them and you won't know anything about it. By declaring your casts as explicit you always know when the cast is being used.

Quote:
Original post by clearly
and when does this happan:
*** Source Snippet Removed ***


The global operator+(const MyInt&, const MyInt&) will be called when any 2 objects of type MyInt are added together. The other 2 operator+ overloads will be called when a MyInt is added to an int or vice-versa.

Quote:
Original post by clearly
and about the others..
MyInt& operator+=(const MyInt& rhs)

cant i just write "MyInt" instade of "MyInt&" ?
what is the diffrent?


Big difference [smile] MyInt& is a reference to a MyInt. References work internally much like pointers do and for simplicity you can think of them like pointers than have non-pointer syntax, but there are some important differences. For instance a reference will always refer to something, you can't have a 'null' reference like you can with pointers.

Quote:
Original post by clearly
and why does the operator+ has to be out of the "class members"?


It doesn't have to be, but it's generally advised to be for the reasons I put in the comments (amongst other reasons).

Share this post


Link to post
Share on other sites
Quote:
Original post by clearly
Quote:
Original post by joanusdmentia
Quote:
Original post by clearly
can any one rewrite my code and show me my mistakes?


Here ya go:

*** Source Snippet Removed ***


thank you very much..
you learned me alot, but i got few things that im not totally understand
like:
*** Source Snippet Removed ***
what does the explicit do, and how does it help me?

and when does this happan:
*** Source Snippet Removed ***
and about the others..
MyInt& operator+=(const MyInt& rhs)

cant i just write "MyInt" instade of "MyInt&" ?
what is the diffrent?
and why does the operator+ has to be out of the "class members"?


The short answers to most of your questions lie in the code comments.

The explicit keyword (which has to be used to declare a constructor) means that such constructor need to be called explicitely. In turn, it means that the compiler is not allowed to silently cast an integer to a MyInt object.
MyInt mi = 10; // ERROR: the constructor have to be called explicitely
MuInt mi2 = MyInt(10); // ok

Automatic conversion, as anything which is done behind the programmer (meaning that the programmer don't have a full control over it) might be dangerous.

For operator+=, returning MyInt instead of MyInt has two different problems:
1) it returns a copy of *this, meaning that it need to run the MyInt constructor (something which is not needed)
2) since it returns a reference, the result of the expression containing i += j is i, not a copy of i; you can use this to chain another operation (such as feeding a function which want a reference with i += j). However, doing so is not very readable, so I'd suggest you to stay away from this.
While point 2 may not be very important, point 1 is still very valid - avoiding the creation of uneeded temporaries is the key to write efficient code. Of course, if the operator+= is inlined, the compiler might be able to avoid the construction of the temporary; but if the operator is not inlined, it will create the temporary even if it is not used.

As for operator+, I suggest you to reread the code comments - they are quite good [smile]

Regards,

Share this post


Link to post
Share on other sites
Quote:
Original post by ProgramMax
Also I agree that += should be implemented in terms of +. You lose the benefit of no temporary, that is true. But you gain exception safety.
Case 1: If += begins modifying itself and an exception occurs half way through, the object is no longer correct and you are not exception safe.
Case 2: If += calls + and the exception occurs in +, the object's state hasn't been altered and you can propogate the exception.

No, you don't gain any exception safety since implementing + via += is already just as exception safe. If an exception is thrown in += and isn't caught in +, you still have both operands in their original state. I'm not sure what you think happens. As well, just as was said earlier, the standard even allows more optimizations if you do it that way as well due to NRVO.


For clarity, you want:


friend your_type operator +( your_type const& left, your_type const& right )
{
your_type result = left;
result += right;
return result; // NRVO
}






Note that this is entirely exception safe and even obeys Abrahams's strong exception guarantee, despite ProgramMax's claims.

Also note that you should prefer the above code to the below, since the above allows for NRVO whereas the below does not, as it uses an unnamed temporary, and it is not elegible for basic RVO as it is not directly returned.


friend your_type operator +( your_type const& left, your_type const& right )
{
// Not optimized and requires += to be a member function
return your_type( left ) += right;
}






Finally, if you have boost installed, just do:



#include <boost/operators.hpp>

class your_type
: boost::addable< your_type >
{
friend your_type& operator +=( your_type& left, your_type const& right )
{
// Implement += here
return *this;
}

// operator + is automatically created and implemented via +=
// as a nonmember and is able to be called via Koenig lookup
};

int main()
{
your_type a, b, c = a + b;
}




Share this post


Link to post
Share on other sites
Quote:
Original post by Polymorphic OOP
Finally, if you have boost installed, just do:

*** Source Snippet Removed ***


That boost trick is very cool. rate++ :-)

Share this post


Link to post
Share on other sites
Quote:
Original post by joanusdmentia
Quote:
Original post by clearly
can any one rewrite my code and show me my mistakes?


Here ya go:

*** Source Snippet Removed ***
There's nothing wrong with putting the definitions of operator + etc in the class. You just have to make it friend.[smile]
Also, if you make them return a const object, you can have the compiler spot bugs like "if (a + b = c)" for you!

The rest of the class after ++ operators...

// You CAN do this from inside the class
friend const MyInt operator+(const MyInt &lhs, const MyInt &rhs)
{
// Code reuse is fun!!
return MyInt(lhs) += rhs;
}

friend const MyInt operator+(int lhs, const MyInt &rhs)
{
return MyInt(lhs) += rhs;
}

// If we provide both this and the above overload
// of operator+() you can keep the cast constructor
// above as explicit (which is a good thing)
friend const MyInt operator+(const MyInt &lhs, int rhs)
{
return MyInt(lhs) += MyInt(rhs);
}

// Play nice with iostreams!
friend std::ostream& operator<<(std::ostream& out, const MyInt& rhs)
{
return out << rhs.GetValue();
}

private:
int value;

};

btw, Thanks for the NRVO tips Polymorphic OOP!!!

Share this post


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

  • Advertisement