views:

290

answers:

5

My colleague keeps telling me of the things listed in comments.

I am confused. Can somebody please demystify these things for me?

class Bar
{
    private int _a;

    public int A
    {
        get { return _a; }
        set { _a = value; }
    }

    private Foo _objfoo;

    public Foo OFoo
    {
        get { return _objfoo; }
        set { _objfoo = value; }
    }

    public Bar(int a, Foo foo)
    {
        // this is a bad idea
        A = a;
        OFoo = foo;
    }

    // MYTHS
    private void Method()
    {
        this.A    //1 -
        this._a   //2 - use this when inside the class e.g. if(this._a == 2)
        A         //3 - use this outside the class e.g. barObj.A
        _a        //4 - 
                  // Not using this.xxx creates threading issues.
    }
}
class Foo
{
    // implementation
}
+9  A: 

The this. is redundant if there isn't a name collision. You only need it when you need a reference to the current object or if you have an argument with the same name as a field.

Threading issues have nothing to do with it. The confusion maybe comes from the fact that most static members are implemented so that they are thread-safe and static members cannot (!) be called with this. since they aren't bound to the instance.

Lucero
Thanks Lucero! This will definitely help in our next <strike>meeting</strike> confrontation ;)
TheMachineCharmer
I think this should be use for good purpose not for confrontation.
Muhammad Kashif Nadeem
@Muhammad Kashif Nadeem: Knowledge is a double edged sword.Its Use is up to the person in possession of it! Just kidding :D
TheMachineCharmer
Static fields and properties are not inherently thread-safe. Methods typically are as long as they don't use any state stored in a static field/property. Properties and methods can be more thread-safe if you correctly use monitors and locks.
Matthew Whited
@Matthew Whited, or a properly structured lock-free algorithm (much faster than using monitors or locks, though only needed in time critical scenarios)
Grant Peters
+2  A: 

The number 2 is a myth that is easily debunked by mentioning automatic properties. Automatic properties allow you to define a property without the backing field which is automatically generated by the compiler. So ask your co-worker what is his opinion about automatic properties.

João Angelo
Very cool! Thanks!
TheMachineCharmer
Beyond that, by using #2 you lose any possible business logic that is in the getter (now or in the future.) Some times I use my getters to apply a default value. Such as if the backing field is null, create a new instance, store to backing field, then return the new value.
Matthew Whited
+8  A: 

"Not using this.xxx creates threading issues"

is a complete myth. Just ask your co-worker to check the generate IL and have him explain why they are the same whether you add this or not.

"use this when inside the class e.g. if(this._a == 2)"

is down to what you want to achieve. What your co-worker seems to be saying is always reference the private field, which does not seem to me sensible. Often you want to access the public property, even inside a class, since the getter may modify the value (for instance, a property of type List may return a new List instance when the list is null to avoid null reference exceptions when accessing the property).

Dan Diplo
AAhha Good point! I am enjoying this!
TheMachineCharmer
"always reference the private property" --> I think that you mean private *field* - it's not a property.
Lucero
You are quite correct Lucero - I've edited my answer to reflect this. Thanks.
Dan Diplo
Additionally, OP, imagine doing `this._a = -2;` with a property like `public int A { get { return _a; } set { if(value < 0) throw new ArgumentOutOfRangeException(); _a = value; } }`
ANeves
+4  A: 

My personal "best practice" is to always use this. Yes it's redundant but it's great way to identify from the first look where the state of the instance is chaged or retrieved when you consider multi-threaded app.

Andrei Taptunov
`this.` is the number one piece of "noise" that I am happy to see in my code. Most of the time I try to remove as much noise as possible (extra braces, nested `if` blocks, etc.)
Matthew Whited
that is also what StyleCop recommends
Stefan Egli
My thoughts exactly!
Andy Shellam
But what to use `this._a` OR `this.A` ?
TheMachineCharmer
I love the constraints StyleCop gives me. StyleCop forces us to always use `this.` on instance fields. I like this 'noise'.
Steven
+3  A: 

It may help to ask your co-worker why he considers these suggestions are best practice? Often people quote best-practice "rules" that they have picked up somewhere without any real understanding of the reasons behind the practices.

As Lucero says, the "this" is not required unless () there is a name collision. However, some people like to include the "this" when it is not strictly required, because they believe it enhances readability / more clearly shows the programmers intentions. In my opinion, this is a matter of personal preference rather than anything else.

As for the "bad idea" in your "Bar" method: Your co-worker may consider this bad practice for the following reason: if the setter method for "A" is altered to have some side effect then A=a; will also produce this side effect, whereas _a = a; will just set the private variable. In my view, best practice is a matter of being aware of the difference rather than prefering one over another.

Finally, the "threading issues" are nonsense - AFAIK "this" has nothing to do with threading.

Kramii