7

Every so often when writing a constructor of a class, I ask myself whether I should be using the initialized member variable or the constructor parameter. Here are two examples to illustrate what I mean:

Constructor Parameter

class Foo {
public:
    Foo(int speed) :
        mSpeed(speed),
        mEntity(speed)
    { }

private:
    int mSpeed;
    Entity mEntity;
}

Member Variable

class Foo {
public:
    Foo(int speed) :
        mSpeed(speed),
        mEntity(mSpeed)
    { }

private:
    int mSpeed;
    Entity mEntity;
}

Further more the same issue arises with using variables in the constructor body.

Constructor Parameter

class Foo {
public:
    Foo(int speed) :
        mSpeed(speed)
    {
        mMonster.setSpeed(speed);
    }

private:
    int mSpeed;
    Monster mMonster;
}

Member Variable

class Foo {
public:
    Foo(int speed) :
        mSpeed(speed)
    {
        mMonster.setSpeed(mSpeed);
    }

private:
    int mSpeed;
    Monster mMonster;
}

I'm aware that it doesn't really matter (except some special cases), that's why I'm rather asking for comments on code design, than what makes it work and what doesn't.

If you need a specific question to work with: What way yields a nice and consistent code design and does one have an (dis)advantage over the other?

Edit: Don't forget the second part of the question. What about variables in the constructor body?

TylerH
  • 20,799
  • 66
  • 75
  • 101
Lukas
  • 2,461
  • 2
  • 19
  • 32
  • As a rule of thumb: Use the initialized member preferably! – πάντα ῥεῖ Feb 07 '14 at 11:12
  • does `ImportantFunction` have a side effect on `mSpeed`? – TemplateRex Feb 07 '14 at 11:16
  • No, it's just a "random" function that can't be called in the init list. – Lukas Feb 07 '14 at 11:18
  • 2
    Somewhat related: Also, don't forget in the case of your first snippet, the order those members are *declared* in the class will dictate their initialization order; *not* the order in the initializer list. If `mEntity` is declared before `mSpeed` in the non-static member list, you'd have a indeterminate value used as an initializer for `mEntity`, even though the order in the initializer list suggests otherwise. – WhozCraig Feb 07 '14 at 11:21
  • @Lukas if it is not related to initialization, why are you calling it in the constructor? Does it affect global variables? does it do post-initialization? – TemplateRex Feb 07 '14 at 11:37
  • @WhozCraig, True, but, IIRC, a warning should be risen by the compiler in that case. – Shoe Feb 07 '14 at 11:46
  • @Jefffrey Both of mine certainly do, though one of them won't without a heightened warning-level, which I find strange. – WhozCraig Feb 07 '14 at 11:57
  • @WhozCraig Yes, I'm aware. I added the members to clear things up. – Lukas Feb 07 '14 at 12:01
  • @TemplateRex I guess you're right that things should not be called there, however it's still a possibility I'd like to think about. Would like some more feedback on the second part of the question. – Lukas Feb 07 '14 at 12:01
  • Since this will sooner-or-later fall down to an opinion question, i'll tell you mine concerning your second part of the question (that apparently many people didn't see). It depends on the function being called. If the type of the parameter is non-const by-reference or by-address , pass the member, otherwise you can pass *either*. If I'm calling a function with a modifiable in/out param I want the right thing modified (my member). I believe my prior-comment concerning member initialization order reflects my opinion on what to do in the member-initializer list (use the param if possible). – WhozCraig Feb 07 '14 at 17:50

6 Answers6

5

I would use the Constructor Parameter, because when using that initializer, the order in which those initializers are executed is dictated by the order in which members were declared, not the order in which they are listed. so, be carefull here.

mlatu
  • 159
  • 1
  • 12
  • Order of initialization is well defined and not "decided" by the compiler. – pmr Feb 07 '14 at 11:23
  • 1
    The compiler doesn't "decide". Its dictated by the standard as member-declared order within the class. It isn't random at all. – WhozCraig Feb 07 '14 at 11:24
  • last time i used initializers the order in which member were declared had no effect on initializers execution. which is why i avoid using them. but ill edit my answer accordingly, if you say there is a standard about this specific thing and that its not the compiler that decides it. better pmr? – mlatu Feb 07 '14 at 11:28
  • C++11 12.6.2 [class.base.init] spells it out. And [see this answer](http://stackoverflow.com/a/4037283/1322972) to a related question if you want more info. If your compiler from whenever did not follow this, be glad you're not using it anymore, because this is paramount to destruction properly working (where things are done precisely in *reverse* order of declaration starting with the last-successful-constructed member and moving backward). – WhozCraig Feb 07 '14 at 11:31
3

I personally prefer to use the constructor parameter in order to avoid using a not initialized yet member variable.

Indeed, in this example:

class Foo {
private:
    int mEntity;
    int mSpeed;
public:
    Foo(int speed) :
        mSpeed(speed),
        mEntity(mSpeed)
    { }
}

the initialization of mEntity will occur before the initialization of mSpeed (because it is declared before). Therefore you will initialize mEntity with a non initialized mSpeed.

--

And inside the constructor body itself, I would also use the constructor parameter because it is a bit more straightforward while debugging to see that you use speed to initialize mMonster and not mSpeed which is itself initialized with speed. Sure it is a minimalistic overhead, but as we can avoid it easily, I think it is better to do it this way.

azerty
  • 698
  • 7
  • 28
  • Yeah that's what my thinking was as well. What about variables in the constructor body? – Lukas Feb 07 '14 at 12:42
  • I would also use the constructor parameter (see my edit for more details) – azerty Feb 07 '14 at 20:04
  • In order to avoid the issue you're adressing gcc complains with a warning: `gcc compiler warning: "will be initialized after/when initialized here"` so if you're initializing `mEntity` after `mSpeed` when it needs to do it in reverse order you'll be prized with the warning. Just turn warnings into errors to avoid this specific issue. – PaperBirdMaster Feb 10 '14 at 10:31
1

I prefer using the member variable for the case where the parameter must be clamped:

class Foo {
public:
    Foo(int speed) :
        mSpeed((speed < 0 ? 0 : speed)),
        mEntity(mSpeed)
    { }
}

That way if the parameter is invalid it is not used to cause subsequent members to be invalid as well.

Otherwise I stick to the parameter variable.

Casey
  • 10,297
  • 11
  • 59
  • 88
0

I would use the constructor parameter. Why? Because of this question. The construtor parameter is clear and readable, you don't need to know a lot about C++ to know what happens. Using a member is error prone if you don't know enough about C++ and is confusing other people on your team that don't have your level of knowledge, even if you do it right.

If in doubt, keep it simple.

nvoigt
  • 75,013
  • 26
  • 93
  • 142
0

You should definitely use constructor parameters. As mentioned, member variables will be initialized in the order they were declared in the header file and not in the order they appear in the initialization list.

Some compilers will warn you if the orders don't match, but using constructor parameters just gives you one less thing to worry about. It's easy, for example, to introduce bugs this way when you edit your class interface. There is nothing to gain by using member variables to initialize other member variables.

jaho
  • 4,852
  • 6
  • 40
  • 66
0

I would use the constructor parameter, also. See simple example:

// foo.h

class Foo
{
    std::unique_ptr<int[]> mBuff;
    int mSize;
public:
explicit    Foo(int size);
   // other methods...
};

// foo.c
Foo::Foo(int size)
  : mSize(size) 
  , mBuff( std::make_unique<int[]>(size) ) // here using mSize is wrong, 
            // because, mSize is not initialized yet.
            // here mSize initialized after mBuff, because it's declarated after mBuff member.
{}

So if you would use member instead of constructor parameter, you may easy create error situation.

Khurshid
  • 2,654
  • 2
  • 21
  • 29