Bad practice to call base methods in derived constructors?

Question

I was just wondering on the most efficient way of setting inherited members was and if the following code is alright to use:

This is the declaration of the base class:

class cEntity{
private:
    int X, Y;
    int Height, Width;
public:
    cEntity();
    cEntity(int x,int y,int h,int w);
    ~cEntity();

    void setX(int x){X=x;};
    void setY(int y){Y=y;};
    void setCoords(int x, int y){X=x;Y=y;};

    void setHeight(int h){Height = h;};
    void setWidth(int w){Width = w;};
    void setArea(int h, int w){Height=h;Width=w;};

    int getX(){return X;};
    int getY(){return Y;};
    //void getXY(int,int);

    int getHeight(){return Height;};
    int getWidth(){return Width;};
    //void getArea(int,int);
};

and here is the constructor of the derived class:

cOrganism::cOrganism () {
    setCoords(0,0);
    setArea(0,0);
    Name = "UNKNOWN";
    Health = 100;
    MaxHealth = 100;
    HealthHiRange =100;
    HealthLoRange = 100;
};

So. Is is alright to call the setCoords() and setArea() in the derived class' constructor?

No it is not bad practice. What is bad practice is [calling virtual methods from your derived constructor][1]. [1]: http://stackoverflow.com/questions/496440/c-virtual-function-from-constructor — Carl, Nov 14 '11 at 03:51
@carleeto: calling virtual methods from a constructor is perfectly all right in C++. it is dangerous in Java. in C++ it is safe and well-defined in a correct program. — Cheers and hth. - Alf, Nov 14 '11 at 05:03

score 7 · Accepted Answer · answered Nov 14 '11 at 03:48

7

It's alright, but you can do much better by calling the base constructor:

cOrganism::cOrganism() : cEntity(0, 0, 0, 0) {
  // other stuff
}

In fact, you should initialize your new, derived members the same way:

cOrganism::cOrganism()
: cEntity(0, 0, 0, 0),
  Name("UNKNOWN"),
  Health(100),
  ...
{
}

(You might also want to read up a bit on general C++ class design: If you expose getters and setters to all your private variables, something isn't quite right. A class is supposed to encapsulate a model, while you're doing essentially the opposite. But that's not a technical error.)

answered Nov 14 '11 at 03:48

Kerrek SB

464,522
92
875
1,084

And how do I set the base constructors arguments in this situation: `cOrganism::cOrganism() : cEntity(0, 0, 0, 0)` As in, what would the syntax look like when creating an object in my code? – Nick Savage Nov 14 '11 at 03:53
What do you mean? You're calling the four-argument version of the base constructor, which I assume would set `X`, `Y`, `Height` and `Width`. You have to implement *that* constructor correctly as well, of course. You create a derived object just with the default constructor: `cOrganism jim;` – Kerrek SB Nov 14 '11 at 03:54
Any argument you want to pass to a constructor has to be a parameter of that constructor... – Keith Layne Nov 14 '11 at 03:56
I understand but when i create an object like this: `cOrganism org3(h, mh, hh, lh, n);` how would i set the base constructor's variables? How does the syntax change? – Nick Savage Nov 14 '11 at 03:56
Oh. Just add a proper constructor for the derived class: `cOrganism(int a, int b, int c, int d, std::string name) : cEntity(a, b, c, d), Name(name), Health(50) { }` – Kerrek SB Nov 14 '11 at 03:58

score 1 · Answer 2 · answered Nov 14 '11 at 03:50

1

Much better to call it like this:

cOrganism::cOrganism () : cEntity(0,0,0,0) {
  Name = "UNKNOWN";
  Health = 100;
  MaxHealth = 100;
  HealthHiRange =100;
  HealthLoRange = 100;
}

Or even better:

cOrganism::cOrganism ()
 : cEntity(0,0,0,0), Name("UNKNOWN"), Health(100), 
   MaxHealth(100), HealthHiRange(100), HealthLoRange(100)
{}

That way the base class members are set as the base class implementation is constructed.

answered Nov 14 '11 at 03:50

Keith Layne

3,688
1
24
28

It seems I am always one step behind @Kerrek SB...Curse you! :) – Keith Layne Nov 14 '11 at 03:54

parapura rajkumar · Answer 3 · 2011-11-14T03:54:55.623

If the default constructor of the base class already intializes it to good values you don't have to do it all.

Otherwise the good solution is something like below.

class A
{
int x;
public:
    A( int xin) :x(xin) {}
};


class B : public A
{
int y;
public:
    B( int xin , int yin ) :A(xin) , y(yin) {}
};

Notice A(xin) in B constructor. This will called pass xin to the A constructor.

If you have something like integers it doesn't really matter what you do. But if A::X was actually a heavy weight object. With your approach A::x will be constructed once with default constructor and then assigned again when setCoords() is called from derived class constructor. My solution will ensure A::x* is only constructed once and that too with all the right value of its parameters.

More details here

Bad practice to call base methods in derived constructors?

3 Answers3