62

How can I write a shorthand of the following scenario?

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

    return _rows;
}
Cody Gray - on strike
  • 239,200
  • 50
  • 490
  • 574
ErrorAgain
  • 755
  • 5
  • 7
  • 14
    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. – Phil K Jul 08 '16 at 13:31
  • 29
    I'm not crazy about this pattern. You have a getter that produces a state change – Bradley Thomas Jul 08 '16 at 13:54
  • http://stackoverflow.com/questions/13876066/is-it-bad-practice-to-have-my-getter-method-change-the-stored-value – Bradley Thomas Jul 08 '16 at 13:55
  • 13
    @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... – KABoissonneault Jul 08 '16 at 13:58
  • 1
    What benefit do you think you'd get by shortening it in any way? – ClickRick Jul 08 '16 at 14:29
  • 10
    @KABoissonneault And as long as this object is never read by multiple threads simultaneously – Tavian Barnes Jul 08 '16 at 18:38
  • 1
    @TavianBarnes Nobody said the real pattern was easy to do correctly, just that – KABoissonneault Jul 08 '16 at 18:49
  • 7
    @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. – Bradley Thomas Jul 08 '16 at 19:04
  • 2
    @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. – Tavian Barnes Jul 08 '16 at 19:16
  • 2
    To return a value that might be null, you write `return someValueThatMightBeNull;`. Want to rephrase the question? – user253751 Jul 10 '16 at 10:56
  • 1
    Just initialize the field in your constructor so it's never `null`. – CodesInChaos Jul 10 '16 at 15:04
  • @CodesInChaos Unless `_rows` is public or is set `null` by any method in the class. Those things would need to be changed as well. – PC Luddite Jul 10 '16 at 16:17
  • 1
    @PCLuddite Both of those would be horrible design, so they should be fixed anyways. – CodesInChaos Jul 10 '16 at 16:27
  • In that specific case, it is often best to make _rows readonly and initialize it in the constructor. – Phil1970 Jul 12 '16 at 01:56
  • And if a property is initialized that way, you need to be aware of possible side effect like the property being evaluated while debugging. – Phil1970 Jul 12 '16 at 01:58

11 Answers11

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.

Zein Makki
  • 29,485
  • 6
  • 52
  • 63
  • 20
    return _rows ?? (_rows = new List()); – eocron Jul 08 '16 at 12:40
  • 1
    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. –  Jul 09 '16 at 17:31
  • 12
    @hvd the documentation provided for ?? is more than clear. I can't see any need to rephrase the **clear** docs in my own words. – Zein Makki Jul 09 '16 at 17:37
  • @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. –  Jul 09 '16 at 17:44
  • 19
    @hvd The OP wanted a *shorthand*, this is exactly what this answer provides. – Zein Makki Jul 09 '16 at 17:52
  • 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. –  Jul 09 '16 at 17:53
  • 5
    @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. – Zein Makki Jul 09 '16 at 17:56
  • 6
    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. – Stefan Jul 09 '16 at 18:40
  • 1
    [C# ignoramus here] Would the 1st example result in any sort of redundant self-assignment, or would it be optimised to nothing in reality? – underscore_d Jul 10 '16 at 09:24
  • 1
    @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. – maaartinus Jul 10 '16 at 12:48
  • 2
    @maaartinus 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. –  Jul 10 '16 at 14:20
  • 1
    @underscore_d 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. – PC Luddite Jul 10 '16 at 16:14
  • @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! – underscore_d Jul 10 '16 at 16:25
  • 1
    @hvd: Don't be absurd. You put your complex initialization into a private method and call it in a single expression just as before. – Kevin Jul 10 '16 at 22:41
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.

Roman Reiner
  • 1,089
  • 1
  • 10
  • 17
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>();
Dmitry Bychenko
  • 180,369
  • 20
  • 160
  • 215
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.

jpmc26
  • 28,463
  • 14
  • 94
  • 146
  • 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. – Mr Lister Jul 10 '16 at 11:55
  • 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. – toddkaufmann Jul 10 '16 at 14:46
  • @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. – jpmc26 Jul 11 '16 at 00:02
12
List<Row> _rows;
public List<Row> Rows => _rows ?? (_rows = new List<Row>());
Sinatr
  • 20,892
  • 15
  • 90
  • 319
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

Richard Ev
  • 52,939
  • 59
  • 191
  • 278
5

Like this for example:

get{ return _rows ?? (_rows = new List<Row>()); }
eocron
  • 6,885
  • 1
  • 21
  • 50
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.

1
return _rows ?? (_rows = new List<Row>());
Matthias
  • 15,919
  • 5
  • 39
  • 84
Dmitriy Kovalenko
  • 3,437
  • 3
  • 19
  • 39
  • 3
    or shorter: return _rows == null? new List : _rows – nosbor Jul 08 '16 at 12:39
  • 3
    @nosbor 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. – Andy Jul 08 '16 at 23:26
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;
    }
Tyler Nichols
  • 196
  • 3
  • 14
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>(); 
}
Abdul Moiz Khan
  • 693
  • 5
  • 13