Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I currently have code that looks like this

private string _stringField;

protected string StringField
{
    get
    {
        if (_stringField == null)
            _stringField = GetStringField();
        return _stringField;
    }
}

ReSharper is suggesting I change the property to:

protected string StringField
{
    get
    {
        return _stringField ?? (_stringField = GetStringField());
    }
}

This isn't an idiom I've seen before and probably would have to think about the first time I saw it; is this something I should be concerned would confuse other people too?

share|improve this question
3  
That and using Lazy<> is how I tend to do my properties when wanting something like this. Just seems more consise to me. As for confusing others? Possibly but it might also teach them something... –  dreza Jan 11 '13 at 19:52
    
@dreza Thanks for pointing out Lazy<>; most of my C# work has been with 3.5 or earlier and this is the first I've seen it. –  Dan Neely Jan 11 '13 at 20:02
    
Even with Lazy<T>, I still tend to use this approach in most cases. Choose whichever feels simpler to you and don't worry about confusing other people. I totally agree with @dreza here: That piece of code is clear enough for someone who saw something like that for the first time to get it in one minute. –  Şafak Gür Jan 12 '13 at 13:28
add comment

2 Answers 2

up vote 13 down vote accepted

?? is well known operator in C#. It doesn't confuse but reduce coding.

Both snippets have same meaning.

But if it is .NET 4.0 onwards, i would rather use Lazy<T>.

private Lazy<string> lazyStringField = new Lazy<string>(GetStringField);
protected string StringField
{
    get
    {
        return lazyStringField.Value;
    }
}
share|improve this answer
    
I'm aware of the ?? operator; what I've never seen before was it being glued together with an assignment statement like that before. –  Dan Neely Jan 11 '13 at 20:01
    
How much slower is this code? –  Leonid Jan 14 '13 at 0:57
3  
While I'd agree that normally ?? doesn't confuse, I'd argue that it's usage here combined with the rarely-used side effect of = which returns the value just set is confusing. –  Bobson Jan 14 '13 at 21:05
    
lazy properties are not supported in CF and should be implemented manually –  hellboy Jan 28 at 14:47
add comment

I noticed that ReSharper's suggestion too and I decided to turn it off. I think that an expression should be used either for its value or for its side-effects, but not both. If you do both, your code will be more confusing. This principle is known as command-query separation.

And I agree with others that using Lazy<T> is even better. If you're still on .Net 3.5 (or older), it shouldn't be that hard to write your own version of that class.

share|improve this answer
    
I've turned off that suggestion as well. Sometimes ReSharper's suggestions make the code less intuitive... –  Bobson Jan 14 '13 at 19:47
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.