5

I'm curious if the following code is valid. Static analysis is giving an error on this constructor.

Summary: Member variable 'A' is initialized by itself.

Summary: Member variable 'B' is initialized by itself.

Summary: Member variable 'C' is initialized by itself.

class Foo
{
public:
    Foo(int A, int B, int C);
private:
    int A;
    int B;
    int C;
}

Foo::Foo(int A, int B, int C) : 
A(A),
B(B),
C(C)
{}

I know that this is not good practice and should probably be changed, however I would like to know if the static analysis warning is a false positive and the member variables would be initialized correctly.

Community
  • 1
  • 1
lcs
  • 4,227
  • 17
  • 36
  • 3
    [Your analyser is wrong.](http://stackoverflow.com/a/6185043/1171191) – BoBTFish May 13 '17 at 15:12
  • 1
    imho it is completely opinion based whether this is bad practice or not. For example one could argue in favor for using it that you dont introduce new names for something when it isnt neccesary at all. – 463035818_is_not_an_ai May 13 '17 at 15:17

3 Answers3

3

Yes, this is a false positive: constructor parameters take precedence over members when evaluating expressions for member initialization list; at the same time, constructor parameters do not participate in the resolution process of member names, so everything works exactly the way you expect.

Your point about this being a bad practice is perfectly valid as well.

Sergey Kalinichenko
  • 714,442
  • 84
  • 1,110
  • 1,523
2

They would be initialized correctly, as instructed in the initialization list.

Indeed, compiling the code, gives:

Georgioss-MacBook-Pro:~ gsamaras$ g++ -Wall main.cpp
main.cpp:6:9: warning: private field 'A' is not used [-Wunused-private-field]
    int A;
        ^
main.cpp:7:9: warning: private field 'B' is not used [-Wunused-private-field]
    int B;
        ^
main.cpp:8:9: warning: private field 'C' is not used [-Wunused-private-field]
    int C;
        ^
3 warnings generated.

where these warnings, are of course, irrelevant.


Further reading: Initializing member variables using the same name for constructor arguments as for the member variables allowed by the C++ standard?


I know that this is not good practice.

It's not a bad one either. The trade off is that you do not introduce new names (such as the capital/lowercase thing one usually sees). I think it's a matter of opinion.

Community
  • 1
  • 1
gsamaras
  • 71,951
  • 46
  • 188
  • 305
2

This is pretty valid, because they're in different scopes. For the member initializer list A(A), the 1st A is in the scope of the class, then the data member A will be found; the 2nd A (which is putted in the parentheses) is in the scope of the constructor, then the parameter A will be found (and hide the data member with the same name).

Quotes from the standard,

$15.6.2/2 Initializing bases and members [class.base.init]:

In a mem-initializer-id an initial unqualified identifier is looked up in the scope of the constructor's class and, if not found in that scope, it is looked up in the scope containing the constructor's definition.

and $15.6.2/15 Initializing bases and members [class.base.init]:

Names in the expression-list or braced-init-list of a mem-initializer are evaluated in the scope of the constructor for which the mem-initializer is specified. [ Example:

class X {
  int a;
  int b;
  int i;
  int j;
public:
  const int& r;
  X(int i): r(a), b(i), i(i), j(this->i) { }
};

initializes X​::​r to refer to X​::​a, initializes X​::​b with the value of the constructor parameter i, initializes X​::​i with the value of the constructor parameter i, and initializes X​::​j with the value of X​::​i; this takes place each time an object of class X is created. — end example ] [ Note: Because the mem-initializer are evaluated in the scope of the constructor, the this pointer can be used in the expression-list of a mem-initializer to refer to the object being initialized. — end note ]

songyuanyao
  • 169,198
  • 16
  • 310
  • 405