28

If the operator= is properly defined, is it OK to use the following as copy constructor?

MyClass::MyClass(MyClass const &_copy)
{
    *this = _copy;
}
MSalters
  • 173,980
  • 10
  • 155
  • 350
gregseth
  • 12,952
  • 15
  • 63
  • 96
  • 10
    Use the [copy-and-swap idiom](http://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom). – GManNickG Sep 06 '10 at 15:28
  • Typically, the copy assignment operator will do some cleanup. If your class has a pointer to dynamically allocated memory, the first thing the copy-assignment operator should do is free that memory. This implementation of the copy constructor would give the copy assignment operator a dangling pointer, which you don't want to delete. – Kevin Nov 08 '14 at 22:40
  • Even if you use smart pointers (in which case delete wouldn't an issue), you'd still be pointlessly default constructing and assigning all the member variables. Just use copy and swap. – Kevin Nov 08 '14 at 22:43

8 Answers8

34

If all members of MyClass have a default constructor, yes.

Note that usually it is the other way around:

class MyClass
{
public:
    MyClass(MyClass const&);     // Implemented
    void swap(MyClass&) throw(); // Implemented
    MyClass& operator=(MyClass rhs) { rhs.swap(*this); return *this; }
};

We pass by value in operator= so that the copy constructor gets called. Note that everything is exception safe, since swap is guaranteed not to throw (you have to ensure this in your implementation).

EDIT, as requested, about the call-by-value stuff: The operator= could be written as

MyClass& MyClass::operator=(MyClass const& rhs)
{
    MyClass tmp(rhs);
    tmp.swap(*this);
    return *this;
}

C++ students are usually told to pass class instances by reference because the copy constructor gets called if they are passed by value. In our case, we have to copy rhs anyway, so passing by value is fine.

Thus, the operator= (first version, call by value) reads:

  • Make a copy of rhs (via the copy constructor, automatically called)
  • Swap its contents with *this
  • Return *this and let rhs (which contains the old value) be destroyed at method exit.

Now, we have an extra bonus with this call-by-value. If the object being passed to operator= (or any function which gets its arguments by value) is a temporary object, the compiler can (and usually does) make no copy at all. This is called copy elision.

Therefore, if rhs is temporary, no copy is made. We are left with:

  • Swap this and rhs contents
  • Destroy rhs

So passing by value is in this case more efficient than passing by reference.

Alexandre C.
  • 55,948
  • 11
  • 128
  • 197
  • 1
    Actually, it doesn't matter if MyClass has a default constructor. Only if data members and base classes have one... – Cătălin Pitiș Sep 06 '10 at 14:13
  • Ok, thanks. I was doing this to avoid code duplication in the implementation of the `operator=` and the copy constructor. With the copy-and-swap idiom the code is duplicated in the copy constructor and the `swap` method. Am I right? – gregseth Sep 08 '10 at 07:42
  • 1
    @gregseth: not quite. The swap operation usually does "shallow" swap, for instance, by swapping just pointers (when appliable). The copy semantic is usually "deep" and thus quite different from the swap semantic. You don't have the code duplication which arises usually with copy ctor/operator= since operator= is implemented *in terms of* the copy ctor. – Alexandre C. Sep 08 '10 at 09:30
  • How do move ctors and assignment fit into this? – Nic Sep 17 '18 at 04:34
  • 1
    @NicHartley : Good one. First, write your move ctor, the most efficiently possible. If it is `noexcept` (that is, it never throws), then you can use `std::swap` instead of implementing `swap` yourself. If it is not `noexcept`, you'll need to think hard about exception safety (this is difficult). The assignment operator stays as is, taking by value and swapping (now with `std::swap`). If now you want move semantics but no copy semantics, then just have the assignment operator take a rvalue reference instead of by value, and swap as usual. – Alexandre C. Sep 17 '18 at 22:32
  • @AlexandreC. I think it'd be worth updating your answer with that, especially now that move semantics have been out for a while and people are expected to use them where appropriate. – Nic Sep 17 '18 at 23:09
  • @AlexandreC. Why member swap and not std::swap? – KeyC0de Aug 28 '22 at 06:50
  • @KeyC0de that was in 2010 and idiomatic C++ wasn't the same as it is now. However this requires swap to be declared, so here I'd have to forward declare the class and swap, or make swap a friend. You can still define a non-member swap afterwards. – Alexandre C. Sep 13 '22 at 06:06
  • @KeyC0de Also note that specializing swap in namespace std isn't allowed so you *don't* use std::swap. You can pick up a swap function in the namespace of your class by ADL in a template context however. – Alexandre C. Sep 13 '22 at 06:08
  • @AlexandreC. What's the issue with doing `using std::swap`? – KeyC0de Sep 13 '22 at 06:45
  • @KeyC0de you can of course, what I'm saying is that you can't specialize std::swap – Alexandre C. Sep 14 '22 at 11:29
11

It is more advisable to implement operator= in terms of an exception safe copy constructor. See Example 4. in this from Herb Sutter for an explanation of the technique and why it's a good idea.

http://www.gotw.ca/gotw/059.htm

Steve Townsend
  • 53,498
  • 9
  • 91
  • 140
5

This implementation implies that the default constructors for all the data members (and base classes) are available and accessible from MyClass, because they will be called first, before making the assignment. Even in this case, having this extra call for the constructors might be expensive (depending on the content of the class).

I would still stick to separate implementation of the copy constructor through initialization list, even if it means writing more code.

Another thing: This implementation might have side effects (e.g. if you have dynamically allocated members).

Cătălin Pitiș
  • 14,123
  • 2
  • 39
  • 62
1

While the end result is the same, the members are first default initialized, only copied after that.

With 'expensive' members, you better copy-construct with an initializer list.

struct C {
   ExpensiveType member;

   C( const C& other ): member(other.member) {}
};



 };
xtofl
  • 40,723
  • 12
  • 105
  • 192
0

I would say this is not okay if MyClass allocates memory or is mutable.

duffymo
  • 305,152
  • 44
  • 369
  • 561
  • 3
    If it's not mutable, then it won't have an `operator=` -- that's a mutating function. Or do I not mean the same thing by mutable as you do? – Steve Jessop Sep 06 '10 at 18:11
0

yes.

personally, if your class doesn't have pointers though I'd not overload the equal operator or write the copy constructor and let the compiler do it for you; it will implement a shallow copy and you'll know for sure that all member data is copied, whereas if you overload the = op; and then add a data member and then forget to update the overload you'll have a problem.

timB33
  • 1,977
  • 16
  • 33
0

@Alexandre - I am not sure about passing by value in assignment operator. What is the advantage you will get by calling copy constructor there? Is this going to fasten the assignment operator?

P.S. I don't know how to write comments. Or may be I am not allowed to write comments.

Manoj R
  • 3,197
  • 1
  • 21
  • 36
  • The usual reference is http://cpp-next.com/archive/2009/08/want-speed-pass-by-value/. I'm still not convinced it's conclusions are always correct. – Steve Jessop Sep 06 '10 at 18:17
  • @Steve: For `operator=`, since you have to do the copy anyway, it cannot be worse. – Alexandre C. Sep 06 '10 at 18:24
  • @Alexandre C: yes it can. For example, if the function isn't inlined then it might result in more copies of the (call to) the copy constructor, bigger binaries, more cache misses, slower. The article mentions this, it isn't hiding anything, and I'm not convinced that it *never* matters whether the caller or the callee does a particular bit of work, on all compilers, ever. – Steve Jessop Sep 06 '10 at 18:51
0

It is technically OK, if you have a working assignment operator (copy operator).

However, you should prefer copy-and-swap because:

  • Exception safety is easier with copy-swap
  • Most logical separation of concerns:
    • The copy-ctor is about allocating the resources it needs (to copy the other stuff).
    • The swap function is (mostly) only about exchanging internal "handles" and doesn't need to do resource (de)allocation
    • The destructor is about resource deallocation
    • Copy-and-swap naturally combines these three function in the assignment/copy operator
Martin Ba
  • 37,187
  • 33
  • 183
  • 337