Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

Sometimes I write constructor code like

class X
  X( const int numberOfThingsToDo ) :
    numberOfThingsToDo( numberOfThingsToDo )

  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#)?

share|improve this question

closed as primarily opinion-based by durron597, Snowman, GlenH7, Ixrec, gnat Aug 15 '15 at 14:17

Many good questions generate some degree of opinion based on expert experience, but answers to this question will tend to be almost entirely based on opinions, rather than facts, references, or specific expertise.If this question can be reworded to fit the rules in the help center, please edit the question.

up vote 11 down vote accepted

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.

share|improve this answer

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.

share|improve this answer
Wait, so "all" doesn't actually mean all? (Or would that be starting another religious war?) – Mason Wheeler Oct 8 '12 at 17:49
+10 for the "don't fight the small stuff" and "don't start programming religion wars" – Jennifer S Oct 8 '12 at 19:08
@MasonWheeler - Yep. Shades of Bill Clinton ("it depends on what the meaning of the word is is"). From the gcc documentation, "Note that some warning flags are not implied by -Wall." So what does "all" mean if it doesn't mean "all"? It means (quoting again) "all the warnings about constructions that some users consider questionable ..." (whatever that means). It doesn't mean -Wextra, -Wold-style-cast, -Woverloaded-virtual, or -Wshadow, for example. There are quite a few other warnings that -Wall does not catch. – David Hammen Oct 8 '12 at 21:22
@MasonWheeler: The utility of an option to enable all possible warnings for any purpose other than compiler testing would be greatly impaired by having the compiler support warnings about things which would be necessary in some programs but highly suspicious in others. Since it's often useful to be able to request warnings for things which would indicate problems in a particular application, even if they would be normal in other applications, that implies that such warnings should be available as options but not be auto-selected under "all". – supercat May 6 '14 at 21:25
@supercat - That would be -Weverything, which is an LLVM (clang, clang++, ...) option. Even the LLVM developers don't recommend that ordinary developers use -Weverything. Too many false positives. – David Hammen May 6 '14 at 22:09

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.

share|improve this answer

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.

share|improve this answer
Compiler warns you about this automatically: g++ -c -Wall -Wextra -Werror -ansi -pedantic pp.cpp => pp.cpp:4: warning: unused parameter ‘a_’ – Loki Astari Oct 8 '12 at 19:38
Guess there's some more flags needed for the commandline. – tp1 Oct 9 '12 at 15:27
@tpl: When compiling C++ those are the minimum flags you should be using. Any warnings produced by the compiler are really logical errors in your code and should be fixed. There is no excuse to write code that has any warnings. – Loki Astari Oct 9 '12 at 15:34

Not the answer you're looking for? Browse other questions tagged or ask your own question.