user555265
user555265

Reputation: 493

c# getters setters style

I'm working on some code where there is lots of code like this:

private int x;

public void SetX(int new_x)
{
   this.SetXValue(new_x); 
}

private void SetXValue(int new_x)
{
   this.x = new_x; 
}

and similarly with properties:

private int x;

public int X 
{
    get { return this.GetX(); }
}

private int GetX()
{
    return this.x; 
}

What i don't get is why the need for the private methods which do the actual work, i.e. why not just have methods like this instead:

public void SetX(int new_x) 
{
  this.x = new_x;
}

public int X
{
    get { return this.x; }
}

is it just the other persons personal choice or is there some good reason for using the first way?

(i typed above code manually so sorry if any mistakes but you should hopefully see what i'm trying to say)

Cheers A

Upvotes: 2

Views: 1927

Answers (9)

GETah
GETah

Reputation: 21409

Yes, that is fine only if the SetValue is private or protected and is doing more than just setting a value.

I am working on a project where we do a lot of those things. That is because we are doing more than just setting a value (value checks, state checks etc.) Having a public setter and a public SetValue does not make sense at all and will confuse your consumers as to what setter to use.

Here is another scenario where we use this kind of design:

public abstract class A{
   protected virtual void SetValue(object value);
   public object SomeObject{
      set{SetValue(value);}
   }
}

In this case, we want class A to delegate setting/checking that value to whatever class inheriting from it.

Upvotes: 0

LexyStardust
LexyStardust

Reputation: 1018

There's no reason for code like that as far as I can see. If you're not doing anything with the new values (like processing/checking before storing) and you're writing C# 3.0 you can actually just shorthand it it to this:

public int MyProperty { get; set; }

The compiler creates the backing store for you and you can just reference:

this.MyProperty

...inside your class. You can also create get-only properties like:

public int MyProperty { get; private set; }

All of which I think is pretty neat!

Upvotes: 7

squelos
squelos

Reputation: 1189

I think it must be an old Java developper that did this.

The .Net way is

private int _foo;

public int Foo
{
     get
     {
         return _foo;
     }
     set
     {
          _foo = value;
          dostuff();
     }
 }

Upvotes: 1

Stilgar
Stilgar

Reputation: 23551

One possible reason would be that the properties can have login that should be executed only when the property is set externally and calls from inside the class do not execute the whole logic but only the logic in the private method. Of course it makes no sense to make these methods in advance because introducing them later will not change the contract of the class. Chances are that whoever wrote this code was new to C# and did not understand what properties do.

Upvotes: 1

Florian Greinacher
Florian Greinacher

Reputation: 14786

A general rule of thumb is to use properties for simple get/set operations and get/set methods when there is a relevant part of logic needed to get/set a value (e.g. validation during set or database access during get).

So if the actual code is really as simple as in your example just use properties and do the work in their getters/setters.

If the actual code is more complex replace the properties (at least the setters) by methods as in your third example.

Upvotes: 1

Mike Simmons
Mike Simmons

Reputation: 1298

i may be missing something here, but this looks a bit mad to me!

You can achieve the same by either using automatic properties or properties with backing fields. here's a good description of both: http://weblogs.asp.net/dwahlin/archive/2007/12/04/c-3-0-features-automatic-properties.aspx

Upvotes: 0

Saeb Amini
Saeb Amini

Reputation: 24380

That's very bizarre, there's no justifiable reason for doing that. Please refactor that code. There's also no need for a SetX method as setter can be included in properties. e.g.:

public int X {get; set;}

Upvotes: 0

Patrik Hägne
Patrik Hägne

Reputation: 17141

No, there is no reason for doing this, it looks liks someone was paid by lines of code.

So, yes, you're right, this is just the other persons personal choice, and it's not a very good one.

Upvotes: 1

Bas
Bas

Reputation: 27085

Why don't you use the Getters and Setters directly to implement your logic? I don't understand the need for additional methods unless you have extra parameters that influence the setter's behavior:

    private int myVar;

    public int MyProperty
    {
        get 
        { 
            return myVar; 
        }
        set 
        {
            myVar = value; 
        }
    }

    public void SetMyPropertySpecial(int a, string reason)
    {
        Console.WriteLine("MyProperty was changed because of " + reason);
        this.myVar = a;
    }

Update:

Indeed, this person seems to like having more lines of code, but the structure is utterly useless. Stick to .NET standards using Getters and Setters (see MSDN)

Upvotes: 1

Related Questions