Is it bad practice to use the same name for arguments and members?

Question

Sometimes I write constructor code like

class X
{
public:
  X( const int numberOfThingsToDo ) :
    numberOfThingsToDo( numberOfThingsToDo )
  {
  }

private:
  int numberOfThingsToDo;
};

or in C#

class X
{
  public X( int numberOfThingsToDo )
  {
     this.numberOfThingsToDo = numberOfThingsToDo;
  }

  private int numberOfThingsToDo;
}

I think the main reason is that when I come up with a suitable member name, I see no reason to use a different one for the argument initializing it, and since I'm also no fan of using underscores the easiest is just to pick the same name. After all it's suitable. Is this considered bad practice however? Any drawbacks (apart from shooting yourself in the foot when forgetting the this in C#)?

Answer 1

It doesn't much matter. If there is a universally agreed practice for the language, then follow that. If not, then follow whatever practice is the consensus for your team. If you are working alone, then do as you please.

Distinguishing between local variables and data members should not be a problem. If you have so many variables in scope that it is a problem, then your class is too large.

Answer 2

This is perhaps one of those programming religion type of issues. Some think shadowing, at least in constructors and setters, is the exactly right thing to do do. I even saw it promoted as a "should" practice (but not a "shall") in a couple of coding standards. Others think it is absolutely horrendous. Some coding standards designate this as a "shall not" practice. Personally I don't like the practice, but my personal dislike isn't strong enough that I feel compelled to write it into the standards I write or help write. "Don't fight the small stuff" and "don't start programming religion wars" take precedence over my personal feelings in this regard.

As a side note, GNU's C++ compiler, along with others, provide a -Wshadow option or equivalent that warns about parameter names shadowing data member names. Note: This option is not a one of those enabled by -Wall. Apparently the GNU compiler authors don't want to start a religious war, either.

There is a third option with C++: Use shadowing where appropriate in the declarations of constructors and setters, but use different names in the definitions of those member functions. As far as the compiler is concerned, there's nothing wrong with giving a parameter a different in the declaration (prototype) of a function and the function's definition. I don't particular like this practice, but I have seen it used intentionally, not just accidentally.

Answer 3

Not only is it not bad, in my opinion, I personally prefer it. If the language supports differentiating the member variable from the local variable, name them both the same and use that option (such as this in your case).

To me, it is much easier to understand this,

void setVariable(int varName) { this.varName = varName);

than it is to understand setting the member variable to a local variable with a completely different name.

Like Kevin said, if the language, or your company, has a "best practice" for this already in place, use it to avoid confusion. If not, then I think you're safe using the same name. Most IDE's that are worth their weight will make the distinction obvious.

Answer 4

I've used same name for a long time, and the only really problematic example I've found is the following in c++:

class A { A(int a_) : a(a) { } int a; }; // error, a uninitialized afterwards.

Otherwise it seems to work every time. Some minor name conflicts can happen, and renaming the member to m_a will usually fix it ; but the uninitialized variable in the ctor init is the most problematic aspect.