Copy constructors

[rKrishna]

I was trying to understand the real need for copy constructors. From
literature, the main reason for redfinition of copy constructor in a
program is to allow deep copying; meaning ability to make copies of
classes with members with static or dynamic memory allocation. However
to do this it requires the programmer to override the default copy
constructor, the destructor & operator=.
I wrote a small program to test if i can doa deep copy with the default
copy constructor. I was expecting the code to crash but it worked. any
thoughts?

src code:
-----------------------------------------------
#include <iostream>
using namespace std;
#include <math.h>
#include <stdio.h>
class Point{
private :
int x;
int y;
int* pZ; //class with a pointer to int
public :
Point();
Point(int a,int b,int c);
friend float Get2Norm(Point p1, Point p2);

};
Point:oint(){

}
Point:oint(int a,int b, int c){
x = a;
y = b;
pZ = new int; //dynamic memory allocated
*pZ = c;
}
float Get2Norm(Point p1,Point p2){
return sqrt(pow((p1.x -p2.x),2)+pow((p1.y -
p2.y),2)+pow((*(p1.pZ) - *(p2.pZ)),2));
}
int main(){
Point p1(1,2,3);
Point p2(4,5,6);
Point p4;
p4 = p1; //using the default copy constructor
cout <<"2nd norm p1,p2 "<<Get2Norm(p1,p2)<<endl;
cout <<"2nd norm p4,p2 "<<Get2Norm(p4,p2)<<endl;
}

output:
-------------------

a.out

2nd norm p1,p2 5.19615
2nd norm p4,p2 5.19615

 

[Thomas Tutone]

I was trying to understand the real need for copy constructors.

I think you mean the need for defining a copy constructor, as opposed
to relying on the compiler-synthesized version.

From
literature, the main reason for redfinition of copy constructor in a
program is to allow deep copying; meaning ability to make copies of
classes with members with static or dynamic memory allocation. However
to do this it requires the programmer to override the default copy
constructor, the destructor & operator=.
I wrote a small program to test if i can doa deep copy with the default
copy constructor. I was expecting the code to crash but it worked. any
thoughts?

src code:
-----------------------------------------------
#include <iostream>
using namespace std;
#include <math.h>
#include <stdio.h>
class Point{
private :
int x;
int y;
int* pZ; //class with a pointer to int
public :
Point();
Point(int a,int b,int c);
friend float Get2Norm(Point p1, Point p2);

};
Point:oint(){

}
Point:oint(int a,int b, int c){
x = a;
y = b;
pZ = new int; //dynamic memory allocated
*pZ = c;
}
float Get2Norm(Point p1,Point p2){
return sqrt(pow((p1.x -p2.x),2)+pow((p1.y -
p2.y),2)+pow((*(p1.pZ) - *(p2.pZ)),2));
}
int main(){
Point p1(1,2,3);
Point p2(4,5,6);
Point p4;
p4 = p1; //using the default copy constructor

The above doesn't use the copy constructor, it uses the
(compiler-synthesized) assignment operator.

cout <<"2nd norm p1,p2 "<<Get2Norm(p1,p2)<<endl;
cout <<"2nd norm p4,p2 "<<Get2Norm(p4,p2)<<endl;
}

output:

Your example is perfectly fine, because you don't define a destructor,
and therefore never delete Point:Z. As a result, you have a simple
memory leak. Suppose you fix that, though. What do you think happens
when both p1 and p4 go out of scope? Each one's pZ - which point to
the same memory - would get deleted. Deleting a pointer twice leads to
undefined behavior. Thus, if you define a proper destructor that
cleans up the int you allocate via "new," your program will exhibit
undefined behavior - which means it may work, it may crash, or may do
anything else (the standard example is "it may reformat your
harddrive," although that one seems less likely).

Best regards,

Tom

 

[george.priv]

I was trying to understand the real need for copy constructors. From
literature, the main reason for redfinition of copy constructor in a
program is to allow deep copying; meaning ability to make copies of
classes with members with static or dynamic memory allocation. However
to do this it requires the programmer to override the default copy
constructor, the destructor & operator=.
I wrote a small program to test if i can doa deep copy with the default
copy constructor. I was expecting the code to crash but it worked. any
thoughts?

Your code works only because it has an error and will leak memory.
Correct code should have destructor that deletes pZ.


George Privalov

 

[Victor Bazarov]

I was trying to understand the real need for copy constructors. From
literature, the main reason for redfinition of copy constructor in a
program is to allow deep copying; meaning ability to make copies of
classes with members with static or dynamic memory allocation. However
to do this it requires the programmer to override the default copy
constructor, the destructor & operator=.

Correct. However, the term "deep copying" is misleading. Early C++
programmers were under the impression that the alternative is rather
dull: shallow copying, as in "bitwise". It's not true, because the
semantics of the default (implicit) copy constructor is to copy-construct
all objects, which invokes copy constructors defined for the member types.

I wrote a small program to test if i can doa deep copy with the
default copy constructor. I was expecting the code to crash but it
worked. any thoughts?

src code:
-----------------------------------------------
#include <iostream>
using namespace std;
#include <math.h>
#include <stdio.h>
class Point{
private :
int x;
int y;
int* pZ; //class with a pointer to int
public :
Point();
Point(int a,int b,int c);
friend float Get2Norm(Point p1, Point p2);

};
Point:oint(){

You leave the members *uninitialised*. That's a BAD IDEA(tm).

}
Point:oint(int a,int b, int c){
x = a;
y = b;
pZ = new int; //dynamic memory allocated
*pZ = c;

You really need to use initialisation instead of assignment here...

}
float Get2Norm(Point p1,Point p2){
return sqrt(pow((p1.x -p2.x),2)+pow((p1.y -
p2.y),2)+pow((*(p1.pZ) - *(p2.pZ)),2));
}
int main(){
Point p1(1,2,3);
Point p2(4,5,6);
Point p4;
p4 = p1; //using the default copy constructor
cout <<"2nd norm p1,p2 "<<Get2Norm(p1,p2)<<endl;
cout <<"2nd norm p4,p2 "<<Get2Norm(p4,p2)<<endl;
}

output:

What do you find surprising here? Every time you create a 'Point'
with arguments, you allocate an int and give it a value. Every time
that 'Point' is copied, the pointer is copied along. Access to it
through a different object is no big deal. Two (or more) 'Point'
objects now share the pointer to int, and thus share the int. You
never delete any of the ints, so they keep on living. Memory is
leaking. If the copy changes its [shared] int using the pointer,
the original object will have its 'int' changed as well.

V

 

[Jerry Coffin]

class Point{
private :
int x;
int y;
int* pZ; //class with a pointer to int
public :
Point();
Point(int a,int b,int c);
friend float Get2Norm(Point p1, Point p2);
};

You haven't the kinds of things that typically cause problems. A more
typical class would include a dtor that freed the memory pointed to by
pZ:

Point::~Point() { delete pZ; }

Now, let's consider what happens if we do something like:

int two_points() {
Point a(1,2,3);
Point b(a);
}

When a and b go out of scope, they need to be destroyed so their dtors
are invoked. The first one will be fine -- it deletes the memory pointed
to by pZ. The second is not so fine -- it has a copy of the same
pointer, so it attempts to delete the same memory again. That gives
undefined behavior.

 

[Ron Natalie]

I wrote a small program to test if i can doa deep copy with the default
copy constructor. I was expecting the code to crash but it worked. any
thoughts?

There are two copying operations in C++. Copy-assignment and
copy construction. The first takes an already constructed
object and copies another object to it. The latter creates
a new object from another object.

int main(){
Point p1(1,2,3);
Point p2(4,5,6);
Point p4;
p4 = p1; //using the default copy constructor

No, it does not use the copy constructor. It uses the copy assignment
operator. This is assignment. P4 is already default constructed at
this point.

If you something like:
Point p4(p1)
or passed a point to a function or returned one from a function
by value it would have used a copy constructor.

But your code is still wrong. It didn't do a deep copy. pZ is
the same value in both p1 and p2.

cout <<"2nd norm p1,p2 "<<Get2Norm(p1,p2)<<endl;
cout <<"2nd norm p4,p2 "<<Get2Norm(p4,p2)<<endl;

This fails to detect the deep copy didn't take place.

}

If you had implemented a destructor:
~Point() { delete pZ; }
you would have had undefined behavior at this point. Since the same
pZ existed in both objects, you'd delete the one created in p4's
constructor. Similarly you wouldn't have deleted the one created in
p1's constructor.

Further, if you had attempted to change *p4.pZ or *p1.pZ you would
have found that that changing one changed the other as they were the
same memory location.

 

[rKrishna]

Thanks for the responses.
I ended up printing the value of the pointer pZ and realized , as all
of you had pointed out, that p1.pZ and p4.pZ were pointing to the same
memory locations.Obviously i was not "deep-copying". I therefore
decided to write a copy constructor. I tried two approaches

1. Copy constructor + explicitly overload assignment operator
2. copy constructor (also performs member-member assignment)

class definition
---------------------------------
class Point{
private :
int x;
int y;
int* pZ; //class with a pointer to int
public :
Point();
~Point();
Point(int a,int b,int c);
Point(const Point &p);
void printpZ();
void printX();
friend float Get2Norm(Point p1, Point p2);
void operator=(const Point &p);
//operator= defined only for approach 1.commented for approach
2.
};
code snippet for option 1:
--------------------------------------
void Point:perator=(const Point &p){
x = p.x;
y = p.y;
pZ = new int;
*pZ = *(p.pZ);
}
Point:oint(const Point &p){
*this = p;
}

code snippet for option 2:
--------------------------------------
Point:oint(const Point &p){
x = p.x;
y = p.y;
pZ = new int;
*pZ = *(p.pZ);
}

in either case i construct a new object using the following statements
====================================================
Point p1(1,2,3);
Point p4(p1);

Questions
==============
1. can we discuss the pros and cons of each method?
2. In either case I see the need for an element-by-element copy. So why
not just have a setter. i dont see what copy constructor buys me.
for instance :
class A {
private:
type1 data1;
type2* pdata2;
..................
typen datan;
public:
A();
A(type1 d1, type2* pd2,....typen dn){
data1 = d1;
data2 = new type2;
*data2 = *pd2;
...............................
datan = dn;
}

~A();
void setparams(type1 d1, type2* pd2,....typen dn){
data1 = d1;
data2 = new type2;
*data2 = *pd2;
...............................
datan = dn;
}
}

i could then do something as trivial as
A a1(p1,*p2,...,pn),a2;
a2.setparams(a1.p1,a1.p2.....,a1.pn);

What am i missing?

 

[LR]

2. In either case I see the need for an element-by-element copy. So why
not just have a setter. i dont see what copy constructor buys me.
for instance :
class A {
private:
type1 data1;
type2* pdata2;
.................
typen datan;
public:
A();

I assume that you didn't initialize your data in the default ctor for
brevity?

But let's consider, should you start out with...
A()
:
data1(),
data2( new data2() ),
datan()
{}

or

A()
:
data1()
data2(0),
datan()
{}

Your choice will make a difference in how you have to implement both
your dtor and setparams.

A(type1 d1, type2* pd2,....typen dn){
data1 = d1;
data2 = new type2;
*data2 = *pd2;
...............................
datan = dn;
}

Think about:
A(type1 d1, type2* pd2, typen dn)
:
data1(d1),
data2(new type2(*pd2)),
datan(dn)
{}

Otherwise you'll be constructing defaults of data1, data2 and datan and
then assigning to them.

~A();

The dtor should release any allocated memory. Of course this depends on
how you implement your default ctor.

void setparams(type1 d1, type2* pd2,....typen dn){
data1 = d1;

You had better be sure that data2 didn't already point to something, or
you've got the makings of a leak here.

data2 = new type2;
*data2 = *pd2;
...............................
datan = dn;
}
}

i could then do something as trivial as

It's only trivial if you only need to do it once, and even then...

A a1(p1,*p2,...,pn),a2;
a2.setparams(a1.p1,a1.p2.....,a1.pn);

What am i missing?

For one thing p1 is not a member of A.
If you were to try: s2.setparams(a1.data1... I think you would get an
error from your compiler.

This is inefficient for no gain elsewhere. First you initialize a2 with
whatever the default values are, and then assign new values to a2;

Of course, you could make data1 and all the data members of A public.
But that defeats part of the purpose of making data private to begin with.

Or you could make getter functions for all your data..

type1 A::getData1() const { return data1; }
type2 *A::getData2() const { return data2; }
typen A::getDatan() const { return datan; }

and then

a2.setparams(a1.getData1(), a2.getData2(), a1.getDatan());

Oops. I made a mistake. Ok, not a big deal. I'm sure I'll find it
when I debug the thing.

And do you really want to have to type all those parameters to the
setter everytime you make a copy of your data? It's a lot of work for
no real gain. Besides, having all those things every time you make a
copy of an object presents us with a great opportunity for hard to find
bugs. And also, we might think that making a copy of itself is
someething that should be done by the class. If we ever change the
class, we don't really want to have to change all those calls to
setparams, do we?

That might lead us to make a more clever setparams... Hmmm... perhaps
something like:

void A::setparams(const A &a) { .... }


And then we might well ask, if we're going to do that, why not use a
copy ctor?

A::A(const A &a)
:
data1(a.data1),
data2( new type2( *a.data2 ) ),
datan(a.datan)
{}

We called the copy ctor for data2 in the copy ctor. If you ever make a
class that contains a pointer to an instance of A, A's copy ctor will
come in handy. Particularly because A needs deep copy.

BTW, I think that you might find better solutions to some of these
problems by not using a raw pointer in your class. You may want some
sort of smart pointer. Probably not std::auto_ptr.

LR

 

[Ron Natalie]

--------------------------------------
void Point:perator=(const Point &p){
x = p.x;
y = p.y;
pZ = new int;
*pZ = *(p.pZ);
}
Point:oint(const Point &p){
*this = p;
}

The copy assignment operator is WRONG here. It doesn't
free up the previous value of pZ. It therefore leaks
memory. If it did delete pZ, then the copy constructor
would be wrong because it would call the copy assignment
operator on an uninitialized object and the delete pZ
would be undefined behavior.

Further your operator= doesn't behave like a built in
one because it doesn't return a value.

Point& operator=(const Point& p) {
x = p.x;
y = p.z;
delete pZ;
pZ = new int;
*pZ = *(p.pZ);
return *this;
}
Point(const Point& p) x(p.x), y(p.y), pZ(new int) {
*pZ = *(p.PZ);
}

It's generally spurious to allocate a single object via new. This isn't
Java. If you want a pZ object allocate one. If you want an array,
use std::vector. If you used members that already have proper copying
semantics, you wouldn't need to define the copy operators yourself
(the compiler generated ones would behave appropriately).