Declaring variables in a getter

Question

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?

**Unrelated but important:** Why are you using "var" when you know the exact type of the variable? You are robbing yourself from the advantages of compile-time typechecking. — mg30rg, Jul 29 '16 at 09:56
@OrelEraki ***Maybe***. This could quite plausibly get closed as either "Stub/Example/Hypothetical" Code and/or "Asking us to change what your code does" and/or "Asking for an X/Y review". Not a C# guy so I'm not sure. — Kaz, Jul 29 '16 at 09:58
@mg30rg `var` is implicitly typed, the compiled code is identical. You're not changing anything other than saving some keystrokes. — Charles Mager, Jul 29 '16 at 10:02
@CharlesMager In this code maybe. (Until it is developed further.) In other cases you can save yourself hours of debugging if you write `bool` instead of `var`. For example, if you accidentally type `var irlItIsBoolean = someInt; someOtherINT;` instead of `var irlItIsBoolean = someInt == someOtherINT;`, your code will compile and have an error somewhere else while if you accidentally type `bool irlItIsBoolean = someInt; someOtherINT;` instead of `bool irlItIsBoolean = someInt == someOtherINT;`, your compiler **will** instantaneously show you have mistaken. — mg30rg, Jul 29 '16 at 10:07
@mg30rg if you're making the assumption it's a `bool` later on (which you will be), it won't compile in either case. — Charles Mager, Jul 29 '16 at 10:11
@CharlesMager It is not an assumption, it is a fact. If it isn't a bool, the code above won't compile. Also using `var` is almost always a sign of sloppy coding and creating a code that is hard to maintain. Use of var is only necessary when dealing with anonymous types. — mg30rg, Jul 29 '16 at 10:17
@mg30rg I don't follow. I meant that the use of `bool` or `var` doesn't matter, it won't compile *in either case* if you made that mistake in the code in the question. `var` is certainly not a sign of 'sloppy coding', it's entirely a style choice. I very rarely explicitly specify types, it just adds noise: StyleCop, ReSharper and most answers [to this question](http://stackoverflow.com/questions/41479/use-of-var-keyword-in-c-sharp) advocate the same. — Charles Mager, Jul 29 '16 at 10:26
Var should only be used in case of anonymous types for a [variety](http://www.brad-smith.info/blog/archives/336) of [reasons](https://www.infoq.com/news/2008/05/CSharp-var). It is kind of sad that even toolmakers create tools encouraging the creation of hard-to-maintain code, but we should not. — mg30rg, Jul 29 '16 at 10:28
@mg30rg that's your opinion, and you're entitled to it. I would disagree, as would many others. — Charles Mager, Jul 29 '16 at 10:31
@CharlesMager Also the official opinion of Microsoft: `the use of var does have at least the potential to make your code more difficult to understand for other developers. For that reason, the C# documentation generally uses var only when it is required.` (Implicitly Typed Local Variables (C# Programming Guide), MSDN) — mg30rg, Jul 29 '16 at 10:34

score 6 · Accepted Answer · answered Jul 29 '16 at 09:54

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

They should be cheap

Try to avoid costly calculations or fetching data from databases and things like that.
They should be consistent

Reading the property twice should return the same value both times.
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.

score 1 · Answer 2 · answered Jul 29 '16 at 09:54

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.

score 0 · Answer 3 · answered Jul 29 '16 at 09:54

0

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

answered Jul 29 '16 at 09:54

Ralf Bönning

14,515
5
49
67

score 0 · Answer 4 · answered Jul 29 '16 at 09:54

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?

Declaring variables in a getter

4 Answers4