Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Safe assignment and copy-and-swap idiom

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?

like image 942
Rafael S. Calsaverini Avatar asked May 06 '11 00:05

Rafael S. Calsaverini


1 Answers

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

like image 67
rlc Avatar answered Sep 29 '22 15:09

rlc