Reducing code duplication between operator= and the copy constructor

Question

I have a class that requires a non-default copy constructor and assignment operator (it contains lists of pointers). Is there any general way to reduce the code duplication between the copy constructor and the assignment operator?

i think you should be able to call the copy constructor inside the operator= — San Jacinto, Sep 25 '09 at 13:01
Some answers are here: http://stackoverflow.com/questions/1457842/is-this-good-code-copy-ctor-operator — Johannes Schaub - litb, Sep 25 '09 at 13:03

score 19 · Accepted Answer · answered Sep 25 '09 at 13:04

19

There's no "general way" for writing custom copy constructors and assignment operators that works in all cases. But there's an idiom called "copy-&-swap":

 class myclass
 {
    ...
 public:
    myclass(myclass const&);

    void swap(myclass & with);

    myclass& operator=(myclass copy) {
        this->swap(copy);
        return *this;
    }

    ...
};

It's useful in many (but not all) situations. Sometimes you can do better. A vector or a string could have a better assignment which reuses allocated storage if it was large enough.

answered Sep 25 '09 at 13:04

sellibitze

27,611
3
75
95

2

+1 - good and terse summary of copy-swap i think. Also good idea about reusing storage. – Johannes Schaub - litb Sep 25 '09 at 13:09
3

You might want to point out the subtlety here between your operator= and the more standard const myclass& operator=(const myclass& other); – Bill Sep 25 '09 at 16:33
1

The swap() should probably marked as nothrow. Ditto on Bills comment. Some more explnation would be nice. – Martin York Sep 25 '09 at 16:49
Also note that: > swap ought to be a free function 'void swap(myclass&, myclass&)' > you still have to enumerate all your attributes in swap, instead of enumerating them in the assignment operator – Matthieu M. Sep 29 '09 at 12:45
How does this prevents code duplication since `swap` must be specialized? – gregseth Sep 08 '10 at 07:41
It's not exactly "code duplication" since initialization is not the same as assignment. But Matthieu is right. You obviously have to "enumerate" the members in your swap function. I see no way around this unless you implement your copy ctor in terms of default constructed members + assignment. But in my opinion, this isn't a good solution. Ideally, you don't have to write copy ctor and assignment yourself and ideally, you don't have classes with 99 members. – sellibitze Sep 08 '10 at 13:16
operator=()'s arguments should always be const. – joke Feb 07 '11 at 17:53
Is there anything special about `this->swap(copy)` here, or would `swap(copy)` also work? – kevinarpe Aug 24 '16 at 13:32
1

@kevinarpe: Nothing special. `swap(copy)` also works. Sometimes I write `this->` to make it more explicit that it's a data member or member function. – sellibitze Aug 26 '16 at 11:28

Vijay Mathew · Answer 2 · 2009-09-25T13:56:43.250

17

Factor out the common code to a private member function. A simple (rather contrived) example:

#include <iostream>

class Test
{
public:
  Test(const char* n)
  {
    name = new char[20];
    strcpy(name, n);
  }

  ~Test()
  {
    delete[] name;
  }

  // Copy constructor
  Test(const Test& t)
  {
    std::cout << "In copy constructor.\n";
    MakeDeepCopy(t);
  }

  // Assignment operator
  const Test& operator=(const Test& t)
  {
    std::cout << "In assignment operator.\n";
    MakeDeepCopy(t);
  }

  const char* get_name() const { return name; }

private:
  // Common function where the actual copying happens.
  void MakeDeepCopy(const Test& t)
  {        
    strcpy(name, t.name);
  }

private:
  char* name;
};

int
main()
{
  Test t("vijay");
  Test t2(t); // Calls copy constructor.
  Test t3(""); 
  t3 = t2; // Calls the assignment operator.

  std::cout << t.get_name() << ", " << t2.get_name() << ", " << t3.get_name() << '\n';

  return 0;
}

edited Sep 25 '09 at 13:56

answered Sep 25 '09 at 13:01

Vijay Mathew

26,737
4
62
93

1

+1 - this is almost always the best way to handle code duplication within a class. – Eric Petroelje Sep 25 '09 at 13:04
3

memory leak! MakeDeepCopy ignores the possibility of name already pointing to allocated memory. – sellibitze Sep 25 '09 at 13:12
4

But it's a pity that the copy constructor cannot use member initializers, this way... – xtofl Sep 25 '09 at 13:20
2

Note that `t2 = t2` won't work with the assignment operator as written. (You care about this if you want to sort an array of them, for example.) – dave4420 Sep 25 '09 at 13:29
2

To add a little more: I don't think reuse is applicable. There's a difference between creating a new object and just assigning to it. In the cases where simple member-wise assignment is sufficient, custom copy constructors and assignment operators are not needed. In case you have to do something more advanced (i.e. managing allocated memory) the similarity between copy and assignment is near zero. – sellibitze Sep 25 '09 at 13:31
@sellibitze The only point of the example is to show that whatever behavior that is common to both copy constructor and the assignment operator should be moved to a common function. Ignore all other shortcomings of the code. – Vijay Mathew Sep 25 '09 at 14:00
2

@Vijay: but now you forgot to initialize "name" for copy construction which sort of proves my point. When explicit resource management is involved copy construction and copy assignment do different things. The possibility of reuse is minimal. – sellibitze Sep 25 '09 at 14:31
2

sir, I don't quite get it, but your operator= doesn't return anything, does it? – Alcott Sep 12 '11 at 13:43

dave4420 · Answer 3 · 2009-09-25T14:30:16.643

8

My &My::operator = (My temp)  // thanks, sellibitze
{
    swap (*this, temp);
    return *this;
}

and implement a specialised std::swap<> (My &, My &).

edited Sep 25 '09 at 14:30

answered Sep 25 '09 at 13:05

dave4420

46,404
6
118
152

1

This is now how copy-&-swap should look like. You're explicitly creating a copy which otherwise may be elided. There's a great article about that here: http://cpp-next.com/archive/2009/08/want-speed-pass-by-value/ – sellibitze Sep 25 '09 at 13:10
3

As Dave notices, alexandrescu found about about this 6 years ago already: http://www.ddj.com/cpp/184403855 . Interesting read too! – Johannes Schaub - litb Sep 25 '09 at 13:15
1

errm, incidentally the poster is also called Dave. I'm sorry about the confusion - i meant the Dave Abrahams of cpp-next :) – Johannes Schaub - litb Sep 25 '09 at 13:19
That Alexandrescu article was really helpful. – Dan Hook Sep 25 '09 at 15:51

score 3 · Answer 4 · answered Sep 25 '09 at 20:59

As has already been pointed out by quite a few posters, having operator= create a new object with the copy constructor and then use swap is the common technique used to not have to replicate code in operator=.

That said, I want to point out some a pro and a con of this technique to help you decide whether it is appropriate.

Pro - exception safety

If your object has resource requirements that could cause a throw and assuming that swap will not throw, this technique provides the strong guarantee of exception safety (either the object being assigned to has taken on the value of the other object or it is unchanged).

Con - resource footprint

An issue with this technique is that it requires a complete new object to be created before the old one is released. If your object requires a lot of resources, this can be a problem.

Reducing code duplication between operator= and the copy constructor

4 Answers4

Linked