Safe assignment and copy-and-swap idiom [closed]

Closed. This question needs to be more focused. It is not currently accepting answers.

---

Want to improve this question开发者_如何转开发? Update the question so it focuses on one problem only by editing this post.

Closed 9 months ago.

Improve this question

I'm learning c++ and I recently learned (here in stack overflow) about the copy-and-swap idiom and I have a few questions about it. So, suppose I have the following class using a copy-and-swap idiom, just for example:

class Foo {
private:
  int * foo;
  int size;

public:
  Foo(size_t size) : size(size) { foo = new int[size](); }
  ~Foo(){delete foo;}

  Foo(Foo const& other){
    size = other.size;
    foo = new int[size];
    copy(other.foo, other.foo + size, foo);
  }

  void swap(Foo& other) { 
    std::swap(foo,  other.foo);  
    std::swap(size, other.size); 
  }

  Foo& operator=(Foo g) { 
    g.swap(*this); 
    return *this; 
  }

  int& operator[] (const int idx) {return foo[idx];}
};

My question is, suppose I have another class that have a Foo object as data but no pointers or other resources that might need custom copying or assignment:

class Bar {
private:
  Foo bar;
public:
  Bar(Foo foo) : bar(foo) {};
  ~Bar(){};
  Bar(Bar const& other) : bar(other.bar) {}; 
  Bar& operator=(Bar other) {bar = other.bar;}
};

Now I have a series of questions:

1. Are the methods and constructors as implemented above for the Bar class safe? Having used the copy-and-swap for Foo make me sure that no harm can be done when assigning or copying Bar?

2. Passing the argument by reference in the copy constructor and in swap is mandatory?

3. Is it right to say that when the argument of operator= is passed by value, the copy constructor is called for this argument to generate a temporary copy of the object, and that it is this copy which is then swapped with *this? If I passed by reference in operator= I would have a big problem, right?

4. Are there situations in which this idiom fails to provide complete safety in copying and assigning Foo?

---

As much as possible, you should initialize the members of your class in the initializer list. That will also take care of the bug I told you about in the comment. With this in mind, your code becomes:

class Foo {
private:
  int size;
  int * foo;

public:
  Foo(size_t size) : size(size), foo(new int[size]) {}
  ~Foo(){delete[] foo;} // note operator delete[], not delete

  Foo(Foo const& other) : size(other.size), foo(new int[other.size]) {
    copy(other.foo, other.foo + size, foo);
  }

  Foo& swap(Foo& other) { 
    std::swap(foo,  other.foo);  
    std::swap(size, other.size); 
    return *this;
  }

  Foo& operator=(Foo g) { 
    return swap(g); 
  }

  int& operator[] (const int idx) {return foo[idx];}
};

and

class Bar {
private:
  Foo bar;
public:
  Bar(Foo foo) : bar(foo) {};
  ~Bar(){};
  Bar(Bar const& other) : bar(other.bar) { }
  Bar& swap(Bar &other) { bar.swap(other.bar); return *this; }
  Bar& operator=(Bar other) { return swap(other); }
}

which uses the same idiom throughout

note

as mentioned in a comment on the question, Bar's custom copy constructors etc. are unnecessary, but we'll assume Bar has other things as well :-)

second question

Passing by reference to swap is needed because both instances are changed.

Passing by reference to the copy constructor is needed because if passing by value, you'd need to call the copy constructor

third question

yes

fourth question

no, but it is not always the most efficient way of doing things

---

1 - Are the methods and constructors as implemented above for the Bar class safe? Having used the copy-and-swap for Foo make me sure that no harm can be done when assigning or copying Bar?

Regarding the copy-ctor: that's always safe (all-or-nothing). It either completes (all), or it throws an exception (nothing).

If your class is only made up of one member (i.e. no base classes as well), the assignment operator will be just as safe as that of the member's class. If you have more then one member, the assignment operator will no longer be "all or nothing". The second member's assignment operator could throw an exception, and in that case the object will be assigned "half way". That means you need to implement copy-and-swap again in the new class to get "all or nothing" assignment.

However it will still be "safe", in the sense that you won't leak any resources. And of course the state of each member individually will be consistent - just the state of the new class won't be consistent, because one member was assigned and the other was not.

2 - Passing the argument by reference in the copy constructor and in swap is mandatory?

Yes, passing by reference is mandatory. The copy constructor is the one that copies objects, so it cannot take it's argument by value, since that would mean the argument has to be copied. This would lead to infinite recursion. (The copy-ctor would be called for the argument, which would mean calling the copy-ctor for the argument, which would mean ...). For swap, the reason is another: if you were to pass the argument by value, you could never use swap to really exchange the contents of two objects - the "target" of the swap would be a copy of the object originally passed in, which would be destroyed immediately.

3 - Is it right to say that when the argument of operator= is passed by value, the copy constructor is called for this argument to generate a temporary copy of the object, and that it is this copy which is then swapped with *this? If I passed by reference in operator= I would have a big problem, right?

Yes, that's right. However it's also quite common to take the argument by reference-to-const, construct a local copy and then swap with the local copy. The by reference-to-const way has some drawbacks however (it disables some optimizations). If you're not implementing copy-and-swap though, you should probably pass by reference-to-const.

4 - Are there situations in which this idiom fails to provide complete safety in copying and assigning Foo?

None that I know of. Of course one can always make things fail with multi-threading (if not properly synchronized), but that should be obvious.

---

This is a classic example of where following an idiom leads to unnecessary performance penalties (premature pessimization). This isn't your fault. The copy-and-swap idiom is way over-hyped. It is a good idiom. But it should not be followed blindly.

Note: One of the most expensive things you can do on a computer is allocate and deallocate memory. It is worthwhile to avoid doing so when practical.

Note: In your example, the copy-and-swap idiom always performs one deallocation, and often (when the rhs of the assignment is an lvalue) one allocation as well.

Observation: When size() == rhs.size(), no deallocation or allocation need be done. All you have to do is a copy. This is much, much faster.

Foo& operator=(const Foo& g) {
    if (size != g.size)
        Foo(g).swap(*this); 
    else
       copy(other.foo, other.foo + size, foo);
    return *this; 
}

I.e. check for the case where you can recycle resources first. Then copy-and-swap (or whatever) if you can't recycle your resources.

Note: My comments do not contradict the good answers given by others, nor any correctness issues. My comment is only about a significant performance penalty.