Conflicts between member names and constructor arg

Question

Possible Duplicate:
Members vs method arguments access in C++

I have a class that has some members, like x, y, width and height. In its constructor, I wouldn't do this:

A::A(int x, int y, int width, int height)
{
    x = x;
    y = y;
    width = width;
    height = height;
}

This doesn't really make sense and when compiled with g++ x, y, width, and height become weird values (e.g. -1405737648).

What is the optimal way of solving these naming conflicts?

Answer 1

You can use initialization lists just fine with the same names:

A::A(int x, int y, int width, int height) :
    x(x),
    y(y),
    width(width),
    height(height)
{
}

An alternative is to use different names, if you don't want to have the same names. Some Hungarian-notation variation comes to mind (I might get some hate for this):

//data members
int x_;
int y_;
int width_;
int height_;
//constructor
A::A(int x, int y, int width, int height) :
    x_(x),
    y_(y),
    width_(width),
    height_(height)
{
}

But there's nothing wrong with the first suggestion.

Answer 2

If you must use assignments in the constructor (as opposed to using a list of initializers, which is preferred) the specific pattern to address this issue is to use this pointer, as follows:

this->a = a;

Answer 3

You can just change the names of the constructor arguments. When you write

A::A(int x, int y, int width, int height)
{
    x = x;
    y = y;
    width = width;
    height = height;
}

then you're assigning the arguments of the constructor to themselves, leaving the actual instance variables uninitialized, that's why you're getting bogus values.

The general solution I suggest (and widely use) is to change the names of the arguments of the constructor method:

A::A(int x_initial, int y_initial, int width_initial, int height_initial)
{
    x = x_initial;
    y = y_initial;
    width = width_initial;
    height = height_initial;
}

Answer 4

If at all possible, it's better to set data members via the initializer list, in which case there's no problem with arguments that shadow member names. Another alternative is to use this->foo = foo; in the body of the constructor. A similar problem exists for setters, but now you can't use the initializer list solution. You're stuck with this->foo = foo; -- or just use different names for arguments and members.

Some people really hate arguments that shadow data members; multiple coding standards explicitly say never to do this. Others think this kind of shadowing, at least for constructors and setters, is the cat's meow. I recall reading one or two coding standards (but I don't recall which) that designated this kind of shadowing as a "should" (but not "shall") practice.

One final option is to use shadowing in the function declaration so as to give readers a hint as to what the function does, but use distinct names in the implementation.

Update: What is "shadowing"?

#include <iostream>

void printi (int i) { std::cout << "i=" << i << "\n"; }

int i = 21; 

int main () {
   printi (i);
   int i = 42; 
   printi (i);
   for (int i = 0; i < 3; ++i) {
      printi (i);
      int i = 10; 
      printi (i);
   }
   printi (i);
}

The innermost declaration of i, int i=10, shadows the variable i declared in the for statement, which in turn shadows the variable i declared at function scope, which in turn shadows the global variable i.

In the problem at hand, the arguments x, y, width, and height to the non-default constructor for class A shadow the member data with the same names as those arguments.

Your width=width; did nothing because the argument width shadows (hides) the data member width. When you have two or more variables with the same name that were declared at different scopes, the winner is always the name with the innermost scope. In general, it is always the name with the innermost scope that wins.

Answer 5

Although you can avoid the problem by using the constructor's initialization list, I suggest following a convention for naming data members, for instance, a trailing _, or a leading m_. Otherwise you are very likely to have name clashes, specially if you have members with names such as x and y.

class A
{
    public:

    A(int x, int y, int width, int height) : x_(x), y_(y), with_(width), height_(height) {}

    int x_;
    int y_;
    int width_;
    int height_;
};