CODEWITHSUNDEEP
c#getter
Stacked
Stacked

Reputation: 7336

Declaring variables in a getter

I have a complex getter as follows

public bool IsOk
{
    get
    {
        return (IsFirstCondition && (IsSecondCondition.Items.First.Item == MyItems.PublicItems.BestItem
        || IsThirdCondition.Collection.EditedItem.IsTheMostUsedItem);
    }
}

For the sake of simplicity and better readability, I want to turn my getter into something like:

public bool IsOk
{
    get
    {
        var isBestItemm = IsSecondCondition.Items.First.Item == MyItems.PublicItems.BestItem;
        var isMostUsedItem = IsThirdCondition.Collection.EditedItem.IsTheMostUsedItem;

        return (IsFirstCondition && (isBestItemm || isMostUsedItem);
    }
}

As far as I know, a getter is meant to return data only not to set/declare/initialize things... Is my simplified getter valid regarding best practices and coding guidelines?

Upvotes: 4

Views: 206

Answers (4)

mg30rg
mg30rg

Reputation: 1349

If I were you I would go with the second solution. Since the scope of your variables doesn't point out from your getter method, they can not be considered as "real" declarations, but they are easier to read and debug. (The compiler will optimize your code for best runtime and memory usage, so don't worry about "extra" variables.) Also why are you using "var" when you know the exact type of the variable?

Upvotes: 0

Ralf Bönning
Ralf Bönning

Reputation: 15415

A Getter should not have side effects and change the state of the object. Otherwise code that increases the readability is welcome.

Upvotes: 0

Lasse V. Karlsen
Lasse V. Karlsen

Reputation: 391346

First of all, the guidelines for properties usually dictate that:

  1. They should be cheap

    Try to avoid costly calculations or fetching data from databases and things like that.

  2. They should be consistent

    Reading the property twice should return the same value both times.

  3. They should not introduce side-effects

    Reading the property changes the underlying object somehow.

If you can avoid that, use any normal "tricks" to refactor the property getter to be

  • More readable
  • More maintainable
  • More reusable (or use more reusable code)

In terms of your actual example, I would definitely declare those variables.

Upvotes: 6

usr
usr

Reputation: 171178

The side-effects in your setter are constrained to local variables. They are not observable to API callers. That makes them harmless.

In other words this does not break the usual expectations one has of properties.

You can do this. There are no concerns with that code.

Upvotes: 1

Related Questions