C++ with copy constructor doesn't work good

Question

I built an object called Attribute, this object has full copy and empty constructor. Then I built another object called Human, which contains the Attribute object. When I try to build the human class (with full constructor) somehow it automatically calls the Attribute copy constructor and I have no idea why.

here is the code:

char** d = new char*[3];    
    d[0] = new char[10];
    d[0] = "aa";
    d[1] = new char[10];
    d[1] = "bb";
    d[2] = new char[10];
    d[2] = "cc";

    Attribute *a = new Attribute(1.7, "blue", "white", "black", d, 3);
    Human *h = new Human("Name", *a);

When I use the debugger and get to this line: new Human("Name", *a); it automatically enters this function:

Attribute::Attribute(Attribute& copy)       
{
    Attribute(copy.height, copy.eyeColor, copy.skinColor, copy.hairColor, copy.diseases, copy.numOfDiseases);
}

and only after this function ends, it starts the Human full constructor...

[Rule of three](http://stackoverflow.com/questions/11024438/rule-of-three-in-c) most probably ... — πάντα ῥεῖ, Jan 09 '14 at 19:04
Do you really need to allocate `Human` and `Attribute` on the heap (i.e. using `new` and keeping a pointer)? My feeling is that you are using C++ as if it where Java and this is not good. — Stefano Sanfilippo, Jan 09 '14 at 19:06
You are leaking memory. As long as you are not familiar with pointers (and arguably even when you are) use std::string and not char*. — nvoigt, Jan 09 '14 at 19:07
You're calling new[], assigning the return value to a pointer, then for some reason replacing the assigned value with a pointer to a string literal, thus leaking memory. `d[0] = new char[10];` `d[0] = "aa"; \\ huh??` — PaulMcKenzie, Jan 09 '14 at 19:23

score 7 · Answer 1 · answered Jan 09 '14 at 19:05

7

Human *h = new Human("Name", *a);
                              ^----- Here it's passing in an Attribute by value

hence calling the Attribute copy constructor.

answered Jan 09 '14 at 19:05

Matt

20,108
1
57
70

Mike Seymour · Answer 2 · 2014-01-09T19:17:33.200

The copy constructor isn't initialising anything; it just creates and destroys a local temporary object.

In C++11, you can delegate the work to another constructor, as you seem to be trying to do:

Attribute::Attribute(Attribute const & copy) :
    Attribute(copy.height, copy.eyeColor, copy.skinColor, copy.hairColor, copy.diseases, copy.numOfDiseases)
{}

Historically, you'd have to duplicate the code of the other constructor, or move it into a function called by both constructors.

You perhaps also want to take the constructor arguments by reference, so that they don't need to be copied:

Human(std::string const & name, Attribute const & attribute);

You should also avoid new unless you really need it. You probably want something more like

Attribute a(1.7, "blue", "white", "black", d, 3);
Human h("Name", a);

When you really do need new (usually because you want the object to outlive the current scope, or sometimes for polymorphism), use RAII management types like smart pointers and containers rather than raw pointers, to ensure the objects are correctly deleted once you've finished with them. Juggling raw pointers is a recipe for memory leaks and other disasters.

score 2 · Answer 3 · answered Jan 09 '14 at 19:27

a - is a pointer *a - is value

Thus, if your constructor for Human takes seconds parameter by value

Human::Human(char* s, Attribute a)

it will copy attribute and use copy constructor for it. If you don't want this behavior, you can pass parameter by pointer.

Human::Human(char* s, Attribute *a)

and call it like this:

Attribute *a = new Attribute(1.7, "blue", "white", "black", d, 3);
Human *h = new Human("Name", a); // this works with pointer now. Pointer will be copied, but the class will remain in the same memory and wouldn't be copied anywhere.

If you think of it, the behavior is similar as with normal values and functions:

void f1(int a){ a++; cout << a; }
void f2(int *a){ *a++; cout << *a; }

int b = 4;
f1(b); // It copies the value of local b to parameter a, increments local parameter a of f1 and prints it; It will print 5
cout << b; // b will remain the same. It will print 4
f2(&b); // It copies pointer, but both pointers &b and a point to the same variable b from this scope, and it's value is not copied 
cout << b; // As we've worked with pointers, it was incremented in f2. It will print 5

Please note that you have to handle responsibilities of all the pointers. If you manually created something, don't forget where it should be deleted and in which cases there could be a leakage. It is much easier to do it with smart_pointers.

C++ with copy constructor doesn't work good

3 Answers3