8

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?

Community
  • 1
  • 1
corazza
  • 31,222
  • 37
  • 115
  • 186
  • 3
    Euh, appending an `a` to the argument names? –  Oct 10 '12 at 15:28
  • `A::A (int xa, int ya, int widtha, int heighta)` – corazza Oct 10 '12 at 15:29
  • if you want to be elegant, *pre*pend the `a`, forming an English indefinite article: `anX, aY, aWidth, aHaight` etc. Obviously, the same identifier can't be used to refer to two different variables in the same scope. See my answer. –  Oct 10 '12 at 15:31
  • Huh? I just chose "a" as it was the first letter of our alphabet, and I appended it instead of prepending it because it looks clearer like that. – corazza Oct 10 '12 at 15:36
  • 1
    I prefer giving member variables names with a single, trailing underscore. ie, `int x_;` – John Dibling Oct 10 '12 at 15:49

5 Answers5

18

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.

Luchian Grigore
  • 253,575
  • 64
  • 457
  • 625
  • 1
    17.4.3.1.2/1: Each name that begins with an underscore is reserved to the implementation for use as a name in the global namespace. – John Dibling Oct 10 '12 at 15:48
  • @JohnDibling why did I think that only applies to macros? Oh well... guess there's one more thing I'll hate when I look back on old code I've written... :D – Luchian Grigore Oct 10 '12 at 15:51
  • what's wrong with Hungarian notation? ;) Although, this is actually just a prefixed name, not really a variant of Hungarian notation. – Zdeslav Vojkovic Oct 10 '12 at 15:52
  • @JohnDibling - 17.4.3.1.2 is "Global names". Data members are not global names. Nonetheless, I don't start data members with an underscore, just because. – David Hammen Oct 10 '12 at 16:07
  • @DavidHammen: I didn't think that's what that meant. I thought that section meant that *any* name in any scope that started with a leading underscore was reserved by the implementation, and the implementation will use it for global names. – John Dibling Oct 10 '12 at 16:11
  • @JohnDibling - The wording isn't crystal clear, but you're probably right. Member names shadow global names in the context of a class, so naming a member with a leading underscore might shadow some global name that the implementation is free to use in any context. It's the shadowing that would violate this rule. Bottom line: Just follow the simple rule of never starting any identifier with an underscore, and never having a double underscore anywhere inside the an identifier. – David Hammen Oct 10 '12 at 17:14
5

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;
Sergey Kalinichenko
  • 714,442
  • 84
  • 1,110
  • 1,523
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.

David Hammen
  • 32,454
  • 9
  • 60
  • 108
  • What exactly do you refer to as "shadowing"? – corazza Oct 10 '12 at 17:45
  • @Bane - What is "shadowing?" Shadowing: (1) A torture device used to test whether CS 101 students understand scoping. (2) When a variable declared at some scope has the same name as a variable declared at some outer scope. (3) What you did with your constructor. (4) A potential problem that is caught by compiling with `-Wshadow`. (5) See my updated answer. – David Hammen Oct 10 '12 at 18:33
2

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_;
};
juanchopanza
  • 223,364
  • 34
  • 402
  • 480
0

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;
}