Why doesn't the copy constructor get called?

Question

I have this code for copying a polygon class. The problem I have is that in the end the vertices point to the original polygon class location. Since the copy constructor does not seem to be invoked. Why is that ?

Polygon::Polygon(const Polygon &aPolyToCopy)
{
int i;
vertices = new Vertex[aPolyToCopy.count];

for (i=0;i<aPolyToCopy.count;i++)
{
    vertices[i].x = aPolyToCopy.vertices[i].x;
    vertices[i].y = aPolyToCopy.vertices[i].y;
}

count = aPolyToCopy.count;
}

In a list template I do this

template <class T, int i>
bool SortedVector<T, i>::add ( const T& v )
{
myClass[myCurrent] = v; //Copy constructor not called ?
myCurrent++;

return true;
}

The template is

 template <class T, int i>
 class SortedVector
 {
 public:
   int maxSize;
   T myClass[i];
   int myCurrent;

   SortedVector();
   ~SortedVector();
   bool add ( const T& v );
};

This is an assignment operation so assignment operator should be called, not the copy constructor. — SomeWittyUsername, Mar 04 '13 at 08:29
I think thats the job of assignment operator, not Copy constructor. — T.Z, Mar 04 '13 at 08:29
You are assigning `myClass[myCurrent] = v;` so assign operator is being called and not copy constructor — Maciek B, Mar 04 '13 at 08:30

score 3 · Accepted Answer · answered Mar 04 '13 at 08:30

3

You're doing an assignment, you don't construct a new object here. If you define a custom copy constructor, you also need to overload operator=

See http://www.learncpp.com/cpp-tutorial/911-the-copy-constructor-and-overloading-the-assignment-operator/ for example.

If you would do something like x = Polygon(y), then your copy constructor would be called (followed by the default operator=). But don't use this workaround, just provide your operator=.

answered Mar 04 '13 at 08:30

Philipp

11,549
8
66
126

"*If you define a custom copy constructor, you also need to overload operator=*" - this is a good design guideline but it has nothing to do with the question asked – SomeWittyUsername Mar 04 '13 at 08:33
2

@icepack How so? The OP's problem comes from the fact they didn't follow this guideline. – Angew is no longer proud of SO Mar 04 '13 at 08:38
"*If you would do something like x = Polygon(y), then your copy constructor would be called (followed by the default operator=)*" - unless copy elision is used by the compiler – SomeWittyUsername Mar 04 '13 at 08:39
@Angew no, the problem is a confusion between copy constructor and assignment operator functionality. – SomeWittyUsername Mar 04 '13 at 08:40

score 1 · Answer 2 · edited May 23 '17 at 10:25

I think a problem in your Polygon class is that you have a vertices data member, which seems to be a raw pointer to a Vertex, used to store a raw array allocated with new[]:

vertices = new Vertex[aPolyToCopy.count];

You may need to overload also operator= (and the destructor), not only the copy constructor (see The Rule of Three); you haven't showed all the code of your Polygon class, so it's not clear if you defined proper copy assignment and destruction.

Note that you will simplify your code if you use a robust RAII container class like std::vector. Just add a "std::vector<Vertex> vertices;" data member instead of a "Vertex* vertices" data member, and std::vector will take care of copy, cleanup, etc. You don't need to do anything: it's all automatically managed by std::vector.

#include <vector>    // for std::vector

class Polygon
{
  std::vector<Vertex> vertices;

public:
  explicit Polygon(size_t vertexCount)
    : vertices(vertexCount) // Build a polygon with specified vertices
  {}

  //  
  // Compiler generated copy constructor, operator= and destructor are fine.
  //
};

In general, in C++ try to build classes assembling together convenient RAII building blocks like std::vector and other direct resource managers.

Why doesn't the copy constructor get called?

2 Answers2