Why I'm getting garbage value after explicitly writing the Copy Constructor?

Question

I have a program which adds two complex numbers using the + operator overloading. The program is:

#include<iostream>
class Complex
{
  int a, b;
  public:
  Complex()
  {
    std::cout<<"\n Normal constructor ";
  }
  void setData(int x, int y)
  {
    a = x; b = y;
  }
  void showData()
  {
    std::cout<<"a = "<<a<<std::endl<<"b = "<<b<<std::endl;
  }
  Complex operator + (Complex c)
  {
    Complex temp;
    temp.a = a + c.a;
    temp.b = b + c.b;
    return temp;
  }
  Complex(Complex &z)
  {
    std::cout<<"\n The copy constructor is called ";
  }
};
int main()
{
  Complex c1, c2, c3;
  c1.setData(2, 3);
  c2.setData(4, 5);
  c3 = c1+c2;
  c3.showData();
  return 0;
}

Here when I do not write the copy constructor explicitly then this program is giving correct output. But after writing copy constructor the output is garbage value and I want to know why is the program producing garbage value?

Output instance:

Normal constructor 
 Normal constructor 
 Normal constructor 
 The copy constructor is called 
 Normal constructor a = -1270468398
b = 32769

Please tell that "what is happening after c3 = c1+2; is executed?"

Others have answered on why your version does not work. I would add that the reason why it works without it is that the compiler tries generates one by default if you don't. And the default one is just fine in this specific case. — François Moisan, Feb 19 '16 at 16:26
Also, why are you adding complex numbers with `c3 = c1.operator+(c2);`? Wasn't the whole point of overloading the `+` operator being able to write `c1 + c2`? :) — Eutherpy, Feb 19 '16 at 16:28

NathanOliver · Accepted Answer · 2016-02-19T16:30:00.003

4

You forgot to copy the members in the copy constructor. If you write your own you have to do that. Since you didn't the members are default initialized and in the case of int that means no initialization happens so you have a garbage value.

Your copy constructor should be

Complex(const Complex &z) : a(z.a), b(z.b)
{
    std::cout<<"\n The copy constructor is called ";
}

You should also take in the object to copy by const& so that you can copy temporary objects. Complex(Complex &z) would not be able to do that.

edited Feb 19 '16 at 16:30

answered Feb 19 '16 at 16:24

NathanOliver

171,901
28
288
402

This answer can even be more great if you tell that what is the reason for getting the garbage value or at what point the data is getting is lost? – Ayushi bhardwaj Feb 19 '16 at 16:28
1

@Ayushibhardwaj Isnt that covered by *Since you didn't the members are default initialized and in the case of `int` that means no initialization happens so you have a garbage value.* – NathanOliver Feb 19 '16 at 16:29
can you please tell what is happening after c3 = c1+2; is executed? – Ayushi bhardwaj Feb 19 '16 at 16:33
@Ayushibhardwaj I already did. Since you never copied the members the copy gets initialized with garbage. Using it is just going to give you more garbage and is undefined behavior. – NathanOliver Feb 19 '16 at 16:35
@OliverQueen : understood. – Ayushi bhardwaj Feb 19 '16 at 16:39
@Oliver : can you tell what `: a(z.a), b(z.b)` is doing? – Ayushi bhardwaj Feb 19 '16 at 16:46
@Ayushibhardwaj http://stackoverflow.com/questions/1711990/what-is-this-weird-colon-member-syntax-in-the-constructor – NathanOliver Feb 19 '16 at 16:48

score 2 · Answer 2 · answered Feb 19 '16 at 16:24

You are not initializing the member data in the copy constructor.

Add code to initialize the members.

Complex(Complex &z) : a(z.a), b(z.b)
{
   std::cout<<"\n The copy constructor is called ";
}

Also, you should make the argument to the copy construct of type const&.

Complex(Complex const& z) : a(z.a), b(z.b)
{
   std::cout<<"\n The copy constructor is called ";
}

score 1 · Answer 3 · edited Feb 19 '16 at 16:50

Your copy constructor is called when passing c2 to the operator +.

There are two options to fix your problem (but both should be fixed anyway):

operator+ should take a const reference instead of a copy of the object:

Complex operator + (const Complex &c)

Your copy constructor should really copy the object (in addition to taking a const reference as parameter):

Complex(const Complex &z) : a (z.a), b(z.b)

There are a few other comments to be done on your code

first I hope it's just an exercise, because std::complex exists
your default constructor should initialize the members
setData / showData seems wrong, you should have meaningful accessors hiding your internals (a complex can be represented by real part / imaginary part but also module / argument etc. ...)
showData should be const (it's not modifying the object, it should be callable on a const object)

I hope this helps.

Sam Daniel · Answer 4 · 2016-02-19T16:49:32.840

In your case, since you don't have any pointers that you want to perform a deep copy, the compiler generated copy constructor is enough.

For more readability, in-case of version >= c++11 you can use this to explicitly

Complex(Complex const&) = default;

To answer your question , what happens when "c3 = c1+c2;"

Function "operator +" will be invoked on C1 object, So here C2 will be copied to the param "Complex c".. So you get the copy constructor called

it is like this to visualize

Comlex operator+(Complex c(c2)/* copy constructor is called here*/)
{
    ...
    return temp.
}

Then operator= function is called on and the temp will be assigned to C3. If you overload operator=, you can see the call to it

That's fine but that doesn't answer why the OP's one doesn't work. — NathanOliver, Feb 19 '16 at 16:40

Why I'm getting garbage value after explicitly writing the Copy Constructor?

4 Answers4