views:

101

answers:

6

I was browsing some documentation for a physics library for XNA and noticed an example someone had used for creating a class for a Car.

This is a pretty simple example:

Class Car
{
   private float gravity;
   private float maxSpeed;

   public Car(float gravity, float maxSpeed)
   {
        this.gravity = gravity;
        this.maxSpeed = maxSpeed;
   }
}

Now when i've created a constructor and set up the assigning of the passed in parameters I would do it this way:

Class Car
{
  private float _gravity;
  private float _maxSpeed;

  public Car(float gravity, float maxSpeed)
  {
       _gravity = gravity;
       _maxSpeed = maxSpeed;
  }
}

Is there any advantage to either approach? I've only come across this a few times but I assume there's a good reason for doing so, I'm just looking for the "best practice" way so to speak.

Thanks!

+2  A: 

Both are common approaches and both are perfectly fine.

Personally I prefer the first because I find the underscores to be ugly. But many people prefer having the underscores because they help to identify that the a variable is a member of the class and not a local variable.

Mark Byers
+4  A: 

Both examples are functionally identical, and both methods are used by different people. It's up to you which naming convention you go with. If you do give the field the same name as the parameter, then you will have to use this to access it.

The important thing is to be consistent in your naming. I personally prefer to access fields with the this keyword, but there are definitely a lot of people around here that like the underscore approach.

Underscore:

If you prefix your private fields with an underscore, then you will always need to use that wherever you refer to it. It is clear (provided that you know the convention) that it is a private field, just from looking at it. Unfortunately, it is also very ugly, but that's just my opinion.

No underscore:

If you do not prefix your private fields with an underscore, then you have the option of referring to it directly or via the this keyword. This means that it possible to forget, and in your example you might assign the argument to itself: gravity = gravity; This should at least generate a warning, though. If you use the StyleCop tool, then it will by default enforce that you always access private fields through the this keyword.

See these similar questions for more answers:

Rich
Great information, thank you!
Jamie Keeling
+3  A: 

With the advent of automatic properties, this is getting pretty popular as well...

public class Car
{
  public Car(float gravity, float speed)
  {
    Gravity = gravity;
    Speed = speed;
  }

  protected float Gravity { get; private set; }
  ...

I personally prefer this approach although I hate that you can't mark an auto-property as readonly.

Rob
Rob: +1 You might be interested in this question http://stackoverflow.com/questions/2480503/is-read-only-auto-imlemented-property-possible
Mark Byers
For some reason its never crossed my mind to do this. I think I have a new preferred method. ;-)
jwsample
+1 - Great idea, I'll definitely give it a try at some point!
Jamie Keeling
+1  A: 

Either is fine. Personally I prefer the this. approach especially with the advent of autoproperties. eg: public string Something {get; set;}. Leaving the constructor argument name the same as the property name gives the user calling the code a very clear indication of where the value will end up.

jwsample
Good idea, thanks!
Jamie Keeling
+1  A: 

If you ever come across Resharper, here are some of its default conventions:

  • Global variables are declared with underscores.

    string _variable1;

  • Global variable which are constants should be declared in upper CamelCase.

    const string Variable1;

  • Properties should always be in upper CamelCase.

    string Variable1 {get;set;}

I may not be a fan of the underscores, but I do see how they help you identify scopes.

Rafael Belliard
A: 

There are some readability issue that "this" can help resolve. I've been in many situations where I'm reading someones code to understand how it works, or how I might derive from it. Reading through a bunch of complex interrelated classes and you find something like

var foo =_FatNumber;

So you start searching for _FatNumber and can't find it defined. That's because it's in the base class. if it's labeled base.Fatnumber you know not to bother looking here, while this.SlimNumber is very explicit. I would advocate explicitly labeling who declares variables (usually the controller of the var). Maybe this is only an issue with more complex derived classes, but it would have saved me a lot of time understanding the class behavior.

for example:

public class FooBase
{
    protected int Foo { get; set; }
}

public class FooDerived : FooBase
{
    int Bar = 9;

    public int GetTheThing(bool something)
    {
        if (somthing)
            return this.Bar;
        else
            return base.Foo;
    }
}
ddm