Is it possible to write a common function that handles both the copy constructor and copy assignment operator?
Asked Answered
O

3

99

Since a copy constructor

MyClass(const MyClass&);

and an = operator overload

MyClass& operator = (const MyClass&);

have pretty much the same code, the same parameter, and only differ on the return, is it possible to have a common function for them both to use?

Orlandoorlanta answered 14/11, 2009 at 15:52 Comment(1)
"...have pretty much the same code..."? Hmm... You must be doing something wrong. Try to minimize the need to use user-defined functions for this and let the compiler do all the dirty work. This often means encapsulating resources in their own member object. You could show us some code. Maybe we have some good design suggestions.Televise
L
136

Yes. There are two common options. One - which is generally discouraged - is to call the operator= from the copy constructor explicitly:

MyClass(const MyClass& other)
{
    operator=(other);
}

However, providing a good operator= is a challenge when it comes to dealing with the old state and issues arising from self assignment. Also, all members and bases get default initialized first even if they are to be assigned to from other. This may not even be valid for all members and bases and even where it is valid it is semantically redundant and may be practically expensive.

An increasingly popular solution is to implement operator= using the copy constructor and a swap method.

MyClass& operator=(const MyClass& other)
{
    MyClass tmp(other);
    swap(tmp);
    return *this;
}

or even:

MyClass& operator=(MyClass other)
{
    swap(other);
    return *this;
}

A swap function is typically simple to write as it just swaps the ownership of the internals and doesn't have to clean up existing state or allocate new resources.

Advantages of the copy and swap idiom is that it is automatically self-assignment safe and - providing that the swap operation is no-throw - is also strongly exception safe.

To be strongly exception safe, a 'hand' written assignment operator typically has to allocate a copy of the new resources before de-allocating the assignee's old resources so that if an exception occurs allocating the new resources, the old state can still be returned to. All this comes for free with copy-and-swap but is typically more complex, and hence error prone, to do from scratch.

The one thing to be careful of is to make sure that the swap method is a true swap, and not the default std::swap which uses the copy constructor and assignment operator itself.

Typically a memberwise swap is used. std::swap works and is 'no-throw' guaranteed with all basic types and pointer types. Most smart pointers can also be swapped with a no-throw guarantee.

Lighter answered 14/11, 2009 at 15:57 Comment(25)
Actually, they are not common operations. While the copy ctor first-time initializes the object's members, the assignment operator overrides existing values. Considering this, alling operator= from the copy ctor is in fact pretty bad, because it first initializes all values to some default just to override them with the other object's values right afterwards.Novelistic
I said common options, not operations. I completely agree that calling operator= from a copy constructor is not good but you only have to look at a reasonable about of real world code to see how common it is.Lighter
Um, didn't you just respond to my explanation?Novelistic
Maybe to "I don't recommend", add "and neither does any C++ expert". Someone might come along and fail to realise that you aren't just expressing a personal minority preference, but the settled consensus opinion of those who've actually thought about it. And, OK, maybe I'm wrong and some C++ expert does recommend it, but personally I'd still lay down the gauntlet for someone to come up with a reference for that recommendation.Terse
@sbi: Yes, I replied to your comment. IMHO both options are very common, but you disagree. Popularity is to some extent a matter of opinion so I'm happy to disagree on that point but I'm interested to know why I've been downvoted which should mean 'wrong or unhelpful' which wasn't my intention.Lighter
@Steve 'onebyone' Jessop: I don't like to sound too prescriptive, people use C++ happily (or not so happily) in many different ways. I added the 'I don't recommend' after re-reading my answer because I felt that my disapproval probably wasn't shown strongly enough by just the contents of the next paragraph.Lighter
Fair enough, I upvoted you already anyway :-). I figure that if something is widely considered best practice, then it's best to say so (and look at it again if someone says it's not really best after all). Likewise if someone asked "is it possible to use mutexes in C++", I wouldn't say "one fairly common option is to completely ignore RAII, and write non-exception-safe code that deadlocks in production, but it's increasingly popular to write decent, working code" ;-)Terse
+1. And i think there is always analysis in need. I think it's reasonable to have an assign member function used by both the copy ctor and assignment operator in some cases (for lightweight classes). In other cases (resource intensive/using cases, handle/body) a copy/swap is teh way to go of course.Unassailable
@litb: Agreed, e.g. if you've just got a couple of basic types in a class then assigning your members won't even have any self-assignment or exception-safe issues in any case so there's little point in sweating about the most robust solution. As soon as your constructors new or do any sort of resource allocation you start to have to consider your options a bit more carefully.Lighter
@litb: If the class is light-weight, does copy-swap do any harm?Novelistic
Yes, since then you don't need a special swap, and i don't see what it buys you wrt exception safety. You would just add one useless function (swap member function), and create one useless copy (since swap/copy would be essentially the same speed here, this is then useless in the real sense). But i might be wrong of course, i'm just don't seeing the point in doing copy-swap then.Unassailable
Of course, it depends on what exactly "light-weight" is - i meant a class that doesn't do dynamic memory management or holding of resources. The need for user defined cctor or assignment ops in this case is rare i think - but it might occasionally come up.Unassailable
Not going to downvote you, but i've never seen a copy constructor implemented in terms of operator= and it sure looks inefficient.Defoliant
@rlbond: I can assure you that it is depressingly common. I've never seen it in code that would otherwise have made me think: 'This is some really clean c++, here!', though.Lighter
In fact, lemme quote Herb Sutter: "If anything, the idiom is exactly backwards: copy construction should be implemented in terms of copy assignment, rather than the reverse." -> gotw.ca/gotw/023.htm . Of course that article is old, but it shows it's not all that crazy of a thought :)Unassailable
Forgive me if my lack of knowledge is showing, but how, in the swap solution, is the initial destruction of the target ensured?Orlandoorlanta
@litb: I was surprised by this so I looked up Item 41 in Exception C++ (which this gotw turned into) and this particular recommendation has gone and he recommends copy-and-swap in its place. Rather sneakily he has dropped "Problem #4: It's Inefficient for Assignment" at the same time.Lighter
@MPelletier: The swap action moves the old state of the target into the temporary copy (as the copy's state is moved into the target). As the initial copy is local to the assignment operator it goes out of scope at the end of the function body, so ensuring that the old state of the target is detroyed.Lighter
@Charles, ah that's good. I expected no different thing from him, now i know he actually did "fix" that :) Nontheless, i don't think it's particularly "bad" to call a common function from both the cctor and copy assignment operator, like some folks here say. Why do it the complex way if an easy solution suffices.Unassailable
@rlbond: Actually, that's exactly how T a(b); was defined waaay back: first call the default constructor and then call the assignment operator. Since this was found to be too inefficient, the notion of a "copy constructor" was introduced in C++ :)Bradleigh
I have a worry about the signature MyClass& operator=(MyClass other). The Visual C++ 2013 compiler appears to still auto generate a MyClass& operator=(const MyClass& other) function implementing memberwise assignment. Should the MyClass& operator=(MyClass other) signature have suppressed the VS generated one?Amity
Isn't the first suggestion, th enot recommended one illegal? If operator = deletes the memory, this will delete a random (uninitialized) pointer. I believe it is not merely "not recommended" but guaranteed to crashBookplate
@Dov: "deletes the memory"? What memory? It sounds like you have a specific example in mind. If operator= is written correctly it shouldn't trigger a crash.Lighter
@charles: operator = is supposed to assume that the object already exists. So if there is a pointer, the object should be deleted. Copy constructor does not. Your second solution where the operator = is written in terms of the copy constructor makes sense. But the first one, how would you write it if the object contains a dynamically allocated pointer?Bookplate
Is there any restriction to write MyClass(other).swap(*this); or this->swap(MyClass(other)); ?Maragaret
N
17

The copy constructor performs first-time initialization of objects that used to be raw memory. The assignment operator, OTOH, overrides existing values with new ones. More often than never, this involves dismissing old resources (for example, memory) and allocating new ones.

If there's a similarity between the two, it's that the assignment operator performs destruction and copy-construction. Some developers used to actually implement assignment by in-place destruction followed by placement copy-construction. However, this is a very bad idea. (What if this is the assignment operator of a base class that called during assignment of a derived class?)

What's usually considered the canonical idiom nowadays is using swap as Charles suggested:

MyClass& operator=(MyClass other)
{
    swap(other);
    return *this;
}

This uses copy-construction (note that other is copied) and destruction (it's destructed at the end of the function) -- and it uses them in the right order, too: construction (might fail) before destruction (must not fail).

Novelistic answered 14/11, 2009 at 16:12 Comment(2)
Should swap be declared virtual?Blew
@Johannes: Virtual functions are used in polymorphic class hierarchies. Assignment operators are used for value types. The two hardly mix.Novelistic
G
-3

Something bothers me about:

MyClass& operator=(const MyClass& other)
{
    MyClass tmp(other);
    swap(tmp);
    return *this;
}

First, reading the word "swap" when my mind is thinking "copy" irritates my common sense. Also, I question the goal of this fancy trick. Yes, any exceptions in constructing the new (copied) resources should happen before the swap, which seems like a safe way to make sure all the new data is filled before making it go live.

That's fine. So, what about exceptions that happen after the swap? (when the old resources are destructed when the temporary object goes out of scope) From the perspective of the user of the assignment, the operation has failed, except it didn't. It has a huge side effect: the copy did actually happen. It was only some resource cleanup that failed. The state of the destination object has been altered even though the operation seems from the outside to have failed.

So, I propose instead of "swap" to do a more natural "transfer":

MyClass& operator=(const MyClass& other)
{
    MyClass tmp(other);
    transfer(tmp);
    return *this;
}

There's still the construction of the temporary object, but the next immediate action is to free all current resources of the destination before moving (and NULLing so they won't be double-freed) the resources of the source to it.

Instead of { construct, move, destruct }, I propose { construct, destruct, move }. The move, which is the most dangerous action, is the one taken last after everything else has been settled.

Yes, destruction fail is a problem in either scheme. The data is either corrupted (copied when you didn't think it was) or lost (freed when you didn't think it was). Lost is better than corrupted. No data is better than bad data.

Transfer instead of swap. That's my suggestion anyway.

Garganey answered 18/1, 2010 at 5:35 Comment(1)
A destructor must not fail, so exceptions upon destruction are not expected. And, I don't get what would be the advantage of moving the move behind destruction, if move is the most dangerous operation? I.e., in the standard scheme, a move failure won't corrupt the old state, whereas your new scheme does. So why? Also, First, reading the word "swap" when my mind is thinking "copy" irritates -> As a library writer, you usually know common practices (copy+swap), and the crux is my mind. Your mind is actually hidden behind the public interface. That's what re-usable code is all about.Zimmer

© 2022 - 2024 — McMap. All rights reserved.