20

I have the following code to test out my understanding of basic pointers in C++:

// Integer.cpp
#include "Integer.h"
Integer::Integer()
{
  value = new int;
  *value = 0;
}

Integer::Integer( int intVal )
{
  value = new int;
  *value = intVal;
} 

Integer::~Integer()
{
  delete value;
}

Integer::Integer(const Integer &rhInt)
{
  value = new int;
  *value = *rhInt.value;
}

int Integer::getInteger() const
{
  return *value;
}

void Integer::setInteger( int newInteger )
{
  *value = newInteger;
}

Integer& Integer::operator=( const Integer& rhInt )
{   
  *value = *rhInt.value;
  return *this;
}

// IntegerTest.cpp
#include <iostream>
#include <cstdlib>
#include "Integer.h"

using namespace std;

void displayInteger( char* str, Integer intObj )
{
  cout << str << " is " << intObj.getInteger() << endl;
}

int main( int argc, char* argv[] )
{
 Integer intVal1;
 Integer intVal2(10);

 displayInteger( "intVal1", intVal1 );
 displayInteger( "intVal2", intVal2 );

 intVal1 = intVal2;

 displayInteger( "intVal1", intVal1 );

 return EXIT_SUCCESS;
}

This code works exactly as expected as is, it prints out:

intVal1 is 0

intVal2 is 10

intVal1 is 10

However if I remove the copy constructor it prints out something like:

intVal1 is 0

intVal2 is 10

intVal1 is 6705152

I don't understand why this is the case. My understanding is that the copy constructor is used when the assignment is to an object that doesn't exist. Here intVal1 does exist, so why isn't the assignment operator called?

LihO
  • 41,190
  • 11
  • 99
  • 167
Lukas Bystricky
  • 1,222
  • 6
  • 16
  • 34
  • 2
    Sorry, I haven't read the whole thing, so not answering, but commenting. I believe the phenomenon observed has to do with the fact that default (automatically generated) copy constructor doesn't allocate memory, but just copies pointer (and yes, considering I haven't read, I may be way off;)). – Michael Krelin - hacker Sep 23 '13 at 21:21
  • `intVal1 = intVal2;` invokes `operator =` on `intVal1`, not the copy constructor. – Hristo Iliev Sep 23 '13 at 21:47

3 Answers3

20

The copy constructor is not used during assignment. Copy constructor in your case is used when passing arguments to displayInteger function. The second parameter is passed by value, meaning that it is initailized by copy contructor.

Your version of copy constructor performs deep copying of data owned by the class (just like your assignment operator does). So, everything works correctly with your version of copy constructor.

If you remove your own copy constructor, the compiler will generate one for you implicitly. The compiler-generated copy constructor will perform shallow copying of the object. This will violate the "Rule of Three" and destroy the functionality of your class, which is exactly what you observe in your experiment. Basically, the first call to displayInteger damages your intVal1 object and the second call to displayInteger damages your intVal2 object. After that both of your objects are broken, which is why the third displayInteger call displays garbage.

If you change the declaration of displayInteger to

void displayInteger( char* str, const Integer &intObj )

your code will "work" even without an explicit copy constructor. But it is not a good idea to ignore the "Rule of Three" in any case. A class implemented in this way either has to obey the "Rule of Three" or has to be made non-copyable.

Community
  • 1
  • 1
AnT stands with Russia
  • 312,472
  • 42
  • 525
  • 765
  • Ok. I think I understand what you're saying. But if I debug my code (without my copy constructor) and break at the line `intVal1 = intVal2`, and ask the values of intVal1 and intVal2 I get 0 and 6299648 respectively. So it seems that only intVal2 has been affected by the shallow copying. Why is this? – Lukas Bystricky Sep 23 '13 at 21:36
  • 1
    @H H: It is just a coincidence or an illusion. Both objects got damaged to the same degree. It just that by coincidence the memory that formerly belonged to `intVal1` retained its former value. Without copy constructor your program exhibits *undefined behavior*. Undefined behavior can take any form. It might even look as if everything is fine. – AnT stands with Russia Sep 23 '13 at 21:38
  • Ok, I can believe that. The only problem I have is that it seems reproducible that intVal = 0 and intVal2 = something else (changes each run). Granted I've only run it 3 times. – Lukas Bystricky Sep 23 '13 at 21:41
  • Rule of Five: When C++11 is used (as is becoming more and more prevalent nowadays), the rule of three is replaced by the "Rule of Five". That means that in addition to a copy constructor, destructor, (copy) assignment operator, now also a move constructor and move assignment operator should be defined. – Piotr99 Sep 23 '13 at 22:20
4

The problem you're experiencing is caused by the default copy constructor, which copies the pointer but doesn't associate it with newly allocated memory (like your implementation of copy constructor does). When you pass object by value, a copy is created and when the execution goes out of scope, this copy is destructed. delete from the destructor invalidates the value pointer of intVal1 object, making it dangling pointer, dereferencing of which causes undefined behavior.

Debug outputs might be used to understand the behavior of your code:

class Integer {
public:

    Integer() {
      cout << "ctor" << endl;
      value = new int;
      *value = 0;
    }

    ~Integer() {
        cout << "destructor" << endl;
        delete value;
    }

    Integer(int intVal) {
      cout << "ctor(int)" << endl;
      value = new int;
      *value = intVal;
    } 

    Integer(const Integer &rhInt) {
      cout << "copy ctor" << endl;
      value = new int;
      *value = *rhInt.value;
    }

    Integer& operator=(const Integer& rhInt){   
      cout << "assignment" << endl;
      *value = *rhInt.value;
      return *this;
    }

    int *value;
};

void foo(Integer intObj) {
    cout << intObj.value << " " << *(intObj.value) << endl;
}

Now output of this code:

Integer intVal1;
Integer intVal2(10);

foo( intVal1 );
foo( intVal2 );

intVal1 = intVal2;

foo( intVal1 );

is:

ctor
ctor(int)
copy ctor
0x9ed4028 0
destructor
copy ctor
0x9ed4038 10
destructor
assignment
copy ctor
0x9ed4048 10
destructor
destructor
destructor

which shows that copy constructor is used when passing objects by value. However, important to notice here is the destructor called upon the return from your function. And if you remove your implementation of copy constructor, then the output is:

ctor
ctor(int)
0x8134008 0
destructor
0x8134018 10
destructor
assignment
0x8134008 135479296
destructor
destructor
destructor

showing that the first copy called delete on the same pointer (pointing to 0x8134008) as was used by third copy later, where the memory pointed by this dangling pointer has been used.

LihO
  • 41,190
  • 11
  • 99
  • 167
3

Think about this call:

displayInteger( "intVal1", intVal1 );

You are creating a copy of intVal1 into the intObj parameter of displayInteger:

void displayInteger( char* str, Integer intObj )
{
  cout << str << " is " << intObj.getInteger() << endl;
}

That copy will be pointing to the same int that intVal1 is. When displayInteger returns, intObj is destroyed, which will cause the int to be destroyed, and the pointer in intVal1 to be pointing to an invalid object. At that point all bets are off (A.K.A. undefined behavior) if you try to access the value. A similar thing happens for intVal2.

At a more general level, by removing the copy constructor, you are violating the Rule of Three, which typically leads to these kinds of problems.

Vaughn Cato
  • 63,448
  • 5
  • 82
  • 132