views:

265

answers:

3

Hi,

I wonder if there is something wrong with the copy constructor function below?

class A  
{  
private:  
int m;  
public:  
A(A a){m=a.m}  
}

Thanks and regards!

+7  A: 

There 3 problems.

First, you have forgot ";" at the end of the m=a.m so your code would not compile.

Second, passing by reference is preferred in most cases where you pass something with size greater than size of a register on your platform.

Third, since you are not going to change the source object, it is better to have it with const. So, finally this is what we have:

A(const A & a) : m(a.m) {}

AlexKR
With regard to your second point: it's not preferred, it's required.
David Thornley
+5  A: 

The problem is that the copy constructor is called for parameter passed by value. So you have to pass the parameter of the copy constructor by reference (usually const reference) if you don't want a non terminating recursion. A minor issue is that you aren't using the initialization list, which is preferable to initialize the member. Fixing both issues:

A(A const& a)
  : m(a.m)
{}  
AProgrammer
Thanks! Why using the initialization list is preferable?
Tim
Giving the value in initialization list saves the extra default initialization that would happen anyway if you omit the list. Doing an assignment in the body of the constructor take extra instructions on top of the initialization.
BennyG
+17  A: 

Two things:

  • Copy constructors must take references as parameters, otherwise they are infinitely recursive (in fact the language won't allow you to declare such constructors)

  • It doesn't do anything the default copy ctor doesn't do, but does it badly - you should use initialisation lists in a copy ctor wherever possible. And if the default copy ctor does what you want, don't be tempted to write a version yourself - you will probably only get it wrong, and you will need to maintain it.

anon
+1 most complete answer
Tronic
Actually it seems, it is a compiler error otherwise. But if it compiled, it should be infinite recursion.
UncleBens
@Uncle, such "copy constructors" are explicitly disallowed. (they demand a diagnostic)
Johannes Schaub - litb
Yeah, I just added a proviso as you guys were posting your comments :-)
anon
Thanks! Is it correct that I can only refer to the class by its pointer or reference not by its object in its definition? I also remember that in node structure for linked list I can define a pointer to the node itself.
Tim
@Tim Those don't sound like questions regarding the copy constructor. You might want to formulate another question, posting some code to illustrate what you are talking about. And do yourself a favour - don't implement yet another linked list.
anon
Thanks. I see. Why "you should use initialisation lists in a copy ctor wherever possible"?
Tim
@Tim Once again, this is a complicated question. Frankly, this should be covered in detail in any good book on C++ - perhaps you are not using a good one?
anon