Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

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

share|improve this question
add comment

4 Answers

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
add comment

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
1  
+10 for the "don't fight the small stuff" and "don't start programming religion wars" –  Jennifer S Oct 8 '12 at 19:08
1  
@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
add comment

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
add comment

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. stackoverflow.com/q/5088460/14065 –  Loki Astari Oct 9 '12 at 15:34
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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