What's wrong with the ?? operator used like this:

Question

So insteed of writing:

if (obj.Collection == null)
    obj.Collection = new Collection();
obj.Collection.Add(something);

I thought of writing:

obj.Collection = obj.Collection ?? new Collection;
obj.Collection.Add(something);

It kind of feels wrong, especially this part "obj.Collection = obj.Collection..."

What do you guys think ?

Regards,

What's the problem? Do you have a problem with `x = x + 1;`? — David Heffernan, Jan 23 '11 at 19:23
Instinctively it feels wrong to me too. But considering David's comment it's probably fine, and the instinct wrong. Now we just need to add the `??=` operator to C# so we can do `obj.Collection ??= new Collection();` — CodesInChaos, Jan 23 '11 at 19:30

score 15 · Accepted Answer · answered Jan 23 '11 at 19:24

15

If I had to choose between these two blocks of code, I would use the former. It's more readable and it's a common pattern. ?? is useful in scenarios where you need to default a value (e.g., DateTime date = nullableDateTimeInstance ?? defaultDate;).

But frankly, I'd try to not end up in situation where I want to add to collection and it's possible that the collection is null. Instead, I'd make sure that the collection is initialized in the constructor or whatever the case may be.

answered Jan 23 '11 at 19:24

jason

236,483
35
423
525

1

Or even do the initialization in the getter of property! – Jahan Zinedine Jan 23 '11 at 19:26
You are very right, but if Collection is a virtual member in a class (let's say a nhibernate entity class). Then you will end up with a warning "Virtual member call in constructor". I prefer the above then this warning. – Calin Jan 23 '11 at 19:35
@Calin: Please give a very simple block of code that reproduces the warning you are describing because I think done properly that should not be the case. – jason Jan 23 '11 at 19:39

score 3 · Answer 2 · answered Jan 23 '11 at 19:25

3

Do you mean:

if (obj.Collection == null)
   {
      obj.Collection = new Collection();
   }
obj.Collection.Add(something);

If so, you can rewrite it as

(obj.Collection = (obj.Collection ?? new Collection())).Add(something);

answered Jan 23 '11 at 19:25

tenor

1,095
6
8

`(obj.Collection ?? (obj.Collection = new Collection())).Add(something);` will work however. – leppie Jan 23 '11 at 19:27
Yeah I realized that shortly after posting the answer and corrected it. – tenor Jan 23 '11 at 19:28
Practice some more :) I can see a compiler error about a mile away by now :p – leppie Jan 23 '11 at 19:29
That's a nightmare to maintain – Sergey Berezovskiy Jan 23 '11 at 19:29
@leppie, your solution will not work if obj.Collection is a read-only property – tenor Jan 23 '11 at 19:30
spoke too soon, and so will mine :P – tenor Jan 23 '11 at 19:32
@tenor: I am not changing the rules :) So according to the OP's code it is not read-only. – leppie Jan 23 '11 at 19:33
1

@leppie, you say there's still something wrong with the code. I can't see where. Can you point it out? – tenor Jan 23 '11 at 19:36
Oh wait, I see you have braces around the assignment. Sorry, my bad :) Mine does avoid reassignment however. – leppie Jan 23 '11 at 19:38

score 0 · Answer 3 · answered Jan 23 '11 at 19:22

The code will emit an unnecessary assignment (of obj.Collection to itself) if obj.Collection is not null, but other than that it is equivalent to the original.

Looks fine to me provided you don't use it in time critical sections. Anyway it's possible the compiler can optimise the assignment away, but I don't know if it would or not.

Perhaps it just feels wrong because the original code is such a common and simple pattern that it feels wrong to change it.

Danko Durbić · Answer 4 · 2011-01-23T20:13:24.950

I'd say it also depends on what the setter of the Collection property does. Consider this:

public Collection Collection
{
    get { return _collection; }
    set
    {
        _collection = value;
        Thread.Sleep( 1000 ); // or some expensive real work
    }
}

In this case, the assignment obj.Collection = obj.Collection ?? new Collection() would be really costly.

However, if you need to create a collection "on demand", it's common to use a similar pattern in the property getter, like this:

public Collection Collection
{
    get { return _collection ?? ( _collection = new Collection() ); } 
}

What's wrong with the ?? operator used like this:

4 Answers4