Why will C++ copy constructor fail?

Question

#include <iostream>
#include <string>

using namespace std;

class A
{
public:
    A() { i=1; j=2;};
    A (A &obj) { i= obj.i+100; j= obj.j+100;};
     int i;
     int j;
};

class B:public A
{
public:
    B():A() {i=10; j=20; k=30;};

    B(A &obj) {  A::A(obj); k=10000; };//

    int k;
};

int main()
{ 
    A dog;
    B mouse(dog);
    cout<<mouse.i<<endl;
    cout<<mouse.k<<endl;

    return 0;
}

I try to write a copy constructor for the derived class that takes advantage of the copy constructor for the base class. I expect that mouse.i should be 101, but in fact the compiling result is 1. The value for mouse.k is 10000, which is expected. I was wondering what's wrong with my code.

Unrelated: member initialization lists are the preferred way of initializing members `A() : i(1), j(2) {}`. — R. Martinho Fernandes, Jul 03 '12 at 15:08
Thanks, and here is just an example. The purpose of the code is to know how to use the copy constructor of the base class. — feelfree, Jul 03 '12 at 15:10
By the way, your derived class doesn't have a copy constructor. It has a constructor that takes the base class, but a copy constructor by definition is a ctor that takes the class itself. — Steve Jessop, Jul 03 '12 at 15:17
Well the answers are all below, but I'd like to add, I would make the copy constructor take a const reference, just as a matter of good practice: `B(const A& obj): A(obj) { k=10000; }` — matiu, Jul 03 '12 at 15:31
@matiu Not just a matter of good practice, taking a `const` reference is *necessary* if you want to be able to copy a temporary object. — Praetorian, Jul 03 '12 at 17:32

score 5 · Answer 1 · answered Jul 03 '12 at 15:10

5

You must use the initialization list to call the parent's constructor (and you should do it for all other members also):

B(A const& obj) : A(obj), k(10000) {}

Additionally, when copying you do not modify the original object, so you should take a const reference to it. That will allow you to copy from constant objects (or through constant references), improving const-correctness.

answered Jul 03 '12 at 15:10

David Rodríguez - dribeas

204,818
23
294
489

1

Taking a const reference will also allow copying temporary objects. – James Kanze Jul 03 '12 at 15:17
I got it. I use VC 2010, and it can compile the original codes. – feelfree Jul 03 '12 at 15:19
@feelfree: Yes, that compiler will accept a lot of broken code; if you think you might ever need portability, then it's a good idea to follow the standard. – Mike Seymour Jul 03 '12 at 15:25

Mike Seymour · Accepted Answer · 2012-07-03T15:21:33.230

5

In this constructor:

B(A &obj) {  A::A(obj); k=10000; };

A::A(obj); does not initialise the base sub-object; instead, it creates a local object also called obj. It's equivalent to A::A obj;, which is equivalant to A obj;. [UPDATE: or possibly it does something else, or possibly it's ill-formed - in any event, it's wrong.]

You want to use an initialiser list:

B(A & obj) : A(obj), k(10000) {}

Also, you almost certainly want the constructor parameters to be A const &, to allow construction from constant objects or temporaries.

edited Jul 03 '12 at 15:21

answered Jul 03 '12 at 15:11

Mike Seymour

249,747
28
448
644

Is `A::A` valid syntax there? – David Rodríguez - dribeas Jul 03 '12 at 15:12
3

@DavidRodríguez-dribeas: Yes, a class name is injected into the class scope. `A::A::A::A::A` is also valid. – Mike Seymour Jul 03 '12 at 15:13
This isn't quite right. `A::A(obj)` doesn't declare anything, it just creates an object with no references. – N_A Jul 03 '12 at 15:14
@Mike gcc doesn't like it without -fpermissive – jrok Jul 03 '12 at 15:14
@mydogisbox: That's exactly what I said: "it creates a local object also called `obj`" – Mike Seymour Jul 03 '12 at 15:14
You mean it hides the parameter with a new local variable called `obj`? – N_A Jul 03 '12 at 15:17
2

A::A(obj) does not create a local object, it creates unnamed temporary by calling a A copy constructor from obj. – Suma Jul 03 '12 at 15:18
1

g++ 4.5.3: `error: ‘base::base’ names the constructor, not the type`. Inside the class, `base` (or `A` in the question) *is* the type, so `base::base` does not refer to the type but the constructor (Admittedly clang++ 3.0 does accept the code... but comeau rejects it, and I am not sure of what the standard has to say to this respect) – David Rodríguez - dribeas Jul 03 '12 at 15:18
@mydogisbox: it's possible that it might actually be ill-formed; one of my compilers accepts it as equivalent to `A obj;`, another rejects it as an attempt to call a constructor directly. I can't really be bothered to figure out which is the case - the answer is simply to fix the code to use an initialiser list. – Mike Seymour Jul 03 '12 at 15:19
@Suma That's what I was thinking was happening. Bad idea either way. – N_A Jul 03 '12 at 15:22
@MikeSeymour Thing is you cannot create a local variable called obj when you have a parameter already named obj - they are both at the same scope and one cannot hide the other. – Suma Jul 03 '12 at 15:26
@Suma: True. The first compiler I tested it on tried to create a local variable (giving an error, as you say); other compilers interpret it differently - another interpreted it as an attempt to directly call the constructor, and presumably the OP's compiler does what you describe, since it apparently compiles for him. Since the compiler writers don't agree on the interpretation, I don't think it's really worth discussing what the strictly correct interpretation should be. – Mike Seymour Jul 03 '12 at 15:29

score 4 · Answer 3 · edited May 23 '17 at 12:27

4

You should initialize the base class like this:

B(A &obj):A(obj) {  k=10000; }

(more on this at What are the rules for calling the superclass constructor?). And a side-note: use const for copy constructor arguments:

A (const A &obj) {...}

EDIT:

The preferred way to initialize instance members is through an initialization list, so your ctor will look like

B(A &obj):A(obj), k(10000) { }

edited May 23 '17 at 12:27

Community

1
1

answered Jul 03 '12 at 15:10

Alexander Pavlov

31,598
5
67
93

Thanks, it works. Could someone explain why my code does not work? – feelfree Jul 03 '12 at 15:13
While this answer is not wrong, I believe Davids or Mikes ones are better, as they promote the use of the initialization list for `B`'s members. Not sure why yours was upvoted and their not. – ereOn Jul 03 '12 at 15:13
@ereOn: Probably a timing issue :) This question is right anyway. – David Rodríguez - dribeas Jul 03 '12 at 15:14
@ereOn is right, the `k(10000)` way of initialization is better. I just pointed out the correct way to initialize the base class. – Alexander Pavlov Jul 03 '12 at 15:14

score 0 · Answer 4 · answered Jul 03 '12 at 15:22

#include <iostream>
#include <string>

using namespace std;

class A
{
public:
A()
{
    i=1;
    j=2;
}

A (A &obj)
{
    i= obj.i+100;
    j= obj.j+100;
}

 int i;
 int j;
};

class B:public A
{
public:
B():A() {i=10; j=20; k=30;}

B(A &obj)
:A(obj)
 {
  //A::A(obj);
  k=10000;
  }//

int k;
};

int main()
{
    A dog;
    B mouse(dog);
    cout<<mouse.i<<endl;
    cout<<mouse.k<<endl;

    return 0;
}

This work for me

B(A &obj)
{
    A::A(obj)
}

Is illegal on gcc compiler

Why will C++ copy constructor fail?

4 Answers4