Jump to content
  • Advertisement
Sign in to follow this  
CmpDev

C++ assignment operator

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

In the current C++ standard there are no move semantics, so with respect to the assignment operator in Factory: is the following safe? if it is not safe why and how would it be done? does it indicate a design flaw or just a language flaw?
 
class foo{...};
class bar{...};
class baz
{
	foo& foo_;
	bar& bar_;
public:
 	bar(foo& f, bar& b):foo_(f),bar_(b){}
 	...
};

class Factory
{
	foo& foo_;
	bar& bar_;
public:
	Factory(foo& f,bar& b):foo_(f),bar(b){}
	Factory(Factory const& rhs):foo_(rhs.foo_),bar(rhs.bar_){}
	Factory& operator=(Factory const& rhs)
	{
		if(this != &rhs)
		{
				this->~Factory();
				new (this) Factory(rhs);
		}
		return *this;
	}
	baz create_baz(){ return baz(foo_,bar_); }
	...
};



Thanks

Share this post


Link to post
Share on other sites
Advertisement
class MyDerivedClass : public Factory { ... };

MyDerivedClass a, b;
a = b; // Whoops, now "a" is a Factory instance ...

Share this post


Link to post
Share on other sites
Yes I never thought about slicing, but if this were an object that was not to have derived instances would it be ok?

I should also point out the this is just demo code and the real code would return shared pointers to base classes when a create method is called.

Share this post


Link to post
Share on other sites
I think it would be cleaner if instead of destructing and re-newing "this", you instead destructed and re-newed a member variable (so that said member variable would not have to be aware that it's being used that way, while the containing object would behave like any other objec)t. A bit like boost::optional but without the optional part (perhaps a boost::variant with a single type).

Share this post


Link to post
Share on other sites
That's funny, I was literally just reading Item 41 in Herb Sutter's Exceptional C++, which presents this exact same question, and a note saying that it appears on various newsgroups every few months, which I now believe :)

Basically although this sometimes works in some situations and more or less achieves what you're trying to do, don't do it because it's "exceedingly poor form and causes no end of problems". In summary, it can slice objects, it's not exception-safe, and it changes normal object lifetimes.

Prefer to write copy assignment in terms of copy construction, like this:


T& T::operator=(const T& other)
{
T temp(other); // do all the work off to the side
Swap(temp); // then 'commit' the work using only nonthrowing operations
return *this;
}




Where 'Swap' is a nonthrowing member function that swaps the guts of two objects. Have a read of either Effective C++ or Exceptional C++ for more details.

Hope that helps :)

Share this post


Link to post
Share on other sites
Quote:
I think it would be cleaner if instead of destructing and re-newing "this", you instead destructed and re-newed a member variable

Yes but the member variable is a reference and you can not reseat a reference and the reference should be the same for all instances created via the factory and between factories.

Quote:
In summary, it can slice objects, it's not exception-safe, and it changes normal object lifetimes.

If the copy constructor was declared explicit then no implicit conversion would happen and therefore it would be an error to try and copy the instance. yes?
Quote:
Where 'Swap' is a nonthrowing member function that swaps the guts of two objects.
How would a copy function be implemented with references?

Share this post


Link to post
Share on other sites
Quote:
Original post by CmpDev
Yes but the member variable is a reference and you can not reseat a reference and the reference should be the same for all instances created via the factory and between factories.
What I meant:

class Functionality
{ /* Look ma, I have a reference member ...
... and no assignment operator. */
};

class Wrapper
{ /* Implements assignment by re-allocating
the "data" member. */

Functionality data;
public:
operator Functionality&() {}
operator const Functionality&() const {} };

Share this post


Link to post
Share on other sites
ToohrVyk If I understand you correctly your solution is like the pimpl idiom, which that instance actually stores the references yes?

Share this post


Link to post
Share on other sites
Quote:
Original post by CmpDev
ToohrVyk If I understand you correctly your solution is like the pimpl idiom, which that instance actually stores the references yes?
Close (the pimpl idiom uses a pointer, so that the definition of the internal implementation doesn't have to be known by other translation units), but yes.

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!