Is there a shorthand way to return values that might be null?
Asked Answered
F

11

62

How can I write a shorthand of the following scenario?

get
{
    if (_rows == null)
    {
        _rows = new List<Row>();
    }

    return _rows;
}
Failure answered 8/7, 2016 at 12:36 Comment(15)
Your code is fine as-is. It can be shortened, but at the cost of readability. It's not worth it to save 3 lines in my opinion.Keramic
I'm not crazy about this pattern. You have a getter that produces a state changeRoseleeroselia
#13876566Roseleeroselia
@BradThomas It's not so bad in some patterns. The example in the question looks a bit like lazy evaluation: the get doesn't change the external state of the object. As long as _rows is not accessed from somewhere else, that is...Lette
What benefit do you think you'd get by shortening it in any way?Diplodocus
@Lette And as long as this object is never read by multiple threads simultaneouslyFari
@TavianBarnes Nobody said the real pattern was easy to do correctly, just thatLette
@Tavian. The problems are more insidious than that. Its generally good for the consumer of a getter to be able to assume that the object is in the same state before and after the property is read. Otherwise surprising side effects can occur, violating the Principle of Least Astonishment.Roseleeroselia
@BradThomas Agreed, but it's definitely possible to do this kind of lazy initialization in a way where you can't observe the state change. If you get that right then it can be a decent optimization.Fari
To return a value that might be null, you write return someValueThatMightBeNull;. Want to rephrase the question?Chiller
Just initialize the field in your constructor so it's never null.Whiteness
@Whiteness Unless _rows is public or is set null by any method in the class. Those things would need to be changed as well.Camelot
@PCLuddite Both of those would be horrible design, so they should be fixed anyways.Whiteness
In that specific case, it is often best to make _rows readonly and initialize it in the constructor.Reduplicative
And if a property is initialized that way, you need to be aware of possible side effect like the property being evaluated while debugging.Reduplicative
F
81

Using null-coalescing operator ( ?? ):

get 
{ 
     _rows = _rows ?? new List<Row>(); 
     return _rows; 
}

OR (less readable):

get { return _rows ?? (_rows = new List<Row>()); }

The ?? operator is called the null-coalescing operator. It returns the left-hand operand if the operand is not null; otherwise it returns the right hand operand.

Fawcett answered 8/7, 2016 at 12:37 Comment(14)
return _rows ?? (_rows = new List<Row>());Sexology
Okay, so you've managed to make the code less readable by basically just cramming everything together on one line in one big expression, and you don't give any explanation of what use this might have. Without an explanation, I don't see how this answer is useful, at least not to anyone who cares about writing maintainable code.Sumption
@hvd the documentation provided for ?? is more than clear. I can't see any need to rephrase the clear docs in my own words.Fawcett
@user3185569 I know what ?? does. That is clear. What isn't clear is what benefit this has over the code the OP already put in the question.Sumption
@hvd The OP wanted a shorthand, this is exactly what this answer provides.Fawcett
Okay, if you only care about making the code shorter, without any regard to whether it remains readable and maintainable, you can shorten it further by changing _rows to a one-character field name and removing the unnecessary spaces.Sumption
@hvd so you don't like ?? ? This is not the place to debate on whether this feature is useful or not in terms of readability.Fawcett
Well, I think in this case it's still quite readable and if you know the ?? operator it shouldn't be a problem to understand it.Philter
[C# ignoramus here] Would the 1st example result in any sort of redundant self-assignment, or would it be optimised to nothing in reality?Leonelleonelle
@hvd This code is perfectly clear (even for a Java programmer like me). The one line expression is a bit harder to understand than any of the eight OP's lines, but it's still pretty simple. It's a stand-alone thing, so there can never be a maintenance problem, as long as the maintainer understand the language syntax.Minaret
@Minaret One obvious maintenance problem is when you need to extend the function to initialise the field to something that cannot be written in a single expression. You can't. You need to first convert it back to what the OP had in the first place.Sumption
@Leonelleonelle I'm not sure if it gets optimized (my windows machine is down, so I can't test it right now), but it definitely results in a redundant self-assignment if it is not optimized at all.Camelot
@PCLuddite I'd be interested to know whether the optimiser catches that if you can try it out later. It seems like the kind of thing that would heavily depend on how the unoptimised instructions are implemented and the order in which the optimiser runs. Or whether it even looks for things like this!Leonelleonelle
@hvd: Don't be absurd. You put your complex initialization into a private method and call it in a single expression just as before.Chromogen
R
41

This is the lazy initialization pattern so the straightforward way would be to use the Lazy<T> class.

class Foo
{
    Lazy<List<Row>> _rows;

    public Foo()
    {
        _rows = new Lazy(() => new List<Row>());
    }

    public List<Row> Rows
    {
        get { return _rows.Value; }
    }
}

This has the additional advantage that it doesn't "pollute" the getter with initialization logic.

Ramsey answered 8/7, 2016 at 17:6 Comment(2)
Though instantiating a list is so cheap that this is silly.Whiteness
I went with what the OP used in his question. I assumed this is just an example and the actual object is more heavy.Ramsey
D
20

I suggest ternary operator

get {
  return _rows == null ? _rows = new List<Row>() : _rows;
}

Or since empty List<Row> doesn't bring much overhead why not get rid of explicit _row field and implement just read-only property (C# 6.0 syntax):

public IList<Row> Rows {get;} = new List<Row>();
Deemphasize answered 8/7, 2016 at 12:39 Comment(2)
The C#6 solution is the way to go.Oregano
For the C# 6, I prefer to generally put new in constructors for easier debugging.Reduplicative
B
18

Here's a better idea: Prevent _rows from ever being null.

Make your constructor initialize the variable:

public MyClass()
{
    this._rows = new List<Row>();
}

and then your property is just

get
{
    return this._rows;
}

Make sure that if you need to "clear" the variable, you always call its Clear method or assign a new empty list instead of assigning null. Maybe encode that operation in a method if you really need to make it clear and consistent throughout the class.

This is much more logical. If your variable should never be null, it should never be null. It also neatly avoids both the conditional and the issue of having a getter modify state.

Blain answered 8/7, 2016 at 21:20 Comment(3)
In the case of an empty list like here, there's not much difference. However, that's not always true in general. What if the initialization of the object takes longer, and you (as the author of the library) are not sure if it's ever requested by the calling program. Then this method will introduce too much overhead and lazy loading is the way to go.Exist
Simpler is better. Users never have to test for null, and you don't have to document it. At least for List (I agree with Lister), and for the first version unless you know otherwise, it's likely premature optimization (or something an automated can do for you). Don't spend time thinking about it.Switch
@MrLister If lazy loading is required because of some performance overhead, then this question chose poorly to replace it with an empty list initialization; a more appropriate choice would be something like MyExpensiveToInitializeClass. This answer applies to the question as written, and assumes that there are no reasons for doing this beyond what is written. If there are, that would be a different question.Blain
R
12
List<Row> _rows;
public List<Row> Rows => _rows ?? (_rows = new List<Row>());
Reremouse answered 8/7, 2016 at 12:44 Comment(0)
P
10

As others have said, you can use the null-coalescing operator in this scenario.

get
{
    return _rows ?? (_rows = new List<Row>());
}

It's worth noting that this is the kind of change that ReSharper is great at suggesting (they call it a quick-fix).

In your example it will put a small squiggle under the if statement. Hovering over it reveals a suggestion for how the code could be changed/simplified.

Convert to '??' expression

A couple of clicks later, and the change is implemented.

enter image description here

Peale answered 8/7, 2016 at 15:1 Comment(0)
S
5

Like this for example:

get{ return _rows ?? (_rows = new List<Row>()); }
Sexology answered 8/7, 2016 at 12:42 Comment(0)
S
3

If you want your code to behave like your current code, lazily initialising your backing field when the property is accessed, then yes, you can make it shorter. You can rename your backing field, as answered already use ?? to put everything in a single expression, and when you have that single expression, use C# 6's new property syntax to avoid writing get and return:

List<Row>_;List<Row> Rows=>_??(_=new List<Row>());

Hopefully, well before you get to this point, you will see that you've turned easy-to-understand code that does exactly what you want into a horrible mess that you would never want to maintain.

Just keep your code exactly as it is. You can make it shorter, as shown, but that doesn't make it any better.

If the problem is that it takes more time to write, because you keep typing the same code over and over, many IDEs provide some feature to insert templates, snippets, or whatever term they use for it. This lets you define something along the lines of

{Type} {Field};
public {Type} {Property} {
  get {
    if ({Field} == null) {
      {Field} = new {Type}();
    }
    return {Field};
  }
}

where your editor will then prompt you for the specific {Type}, {Field}, {Property}, without having to type it again each time.

Sumption answered 9/7, 2016 at 18:14 Comment(0)
C
1
return _rows ?? (_rows = new List<Row>());
Conjuncture answered 8/7, 2016 at 12:37 Comment(2)
or shorter: return _rows == null? new List<Row> : _rowsDeciare
@Deciare both of these continue to return a new instance of a list as they leave _rows unassigned, which is different behavior than the OP is asking for.Institutor
P
0

If you really wanted to shorten it I would just remove the extra brackets.

    get
    {
        if (_rows == null)
            _rows = new List<Row>();

        return _rows;
    }
Pardoner answered 8/7, 2016 at 12:46 Comment(5)
better not, reallyStuckey
I'm not sure why this is getting downvoted. Sure, it doesn't actually do anything apart from shortening the source code, but still. Removing curly braces from one-statement if statements is considered a good thing by many.Exist
@MrLister It's not really any better. It's the same code.Camelot
@PCLuddite It certainly is shorter, is it not? :)Pardoner
This is the best answerMarkettamarkey
B
0

You can do this by any of the following ways:

  • Conditional operator (?:)
  • Null-coalescing operator ( ?? )

With Conditional operator

get {
  return _rows == null ? new List<Row>() : _rows;
}

Null-coalescing operator

get { 
  return _rows ?? new List<Row>(); 
}
Brian answered 30/7, 2016 at 0:50 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.