Ayende @ Rahien

Hi!
My name is Oren Eini
Founder of Hibernating Rhinos LTD and RavenDB.
You can reach me by phone or email:

ayende@ayende.com

+972 52-548-6969

, @ Q c

Posts: 5,972 | Comments: 44,524

filter by tags archive

Explain this code: Answers

time to read 1 min | 170 words

The reason this code is useful?

image

Because it allows you to write this sort of code:

class Program
{
    private static Collection<int> nums;

    static void Main(string[] args)
    {
        nums.Add(1);
        Console.WriteLine(nums.Count);
    }
}

I was aiming for that, and still this code strikes me as wrong.


Comments

Buildstarted

The fact that it's missing an initializer for nums?

Richard

Structs don't require an initialiser as they always have a constructor that will initialise all its fields. The above code is an abuse of this mechanic.

Thomas Levesque

@Richard, it will initialize all fields to their default value, which is null in the case of IList, so you still have to initialize inner when needed (you can't do it in the constructor, since you can't define a default constructor for a struct)

Mads Topro

@Thomas: Maybe, Add method is initializing the inner field before use it.

Thomas Levesque

@Mads, yes, this is what I had in mind ;)

Daniel Grunwald

Ouch, so if I copy the struct after it's initialized, the underlying list will be shared, but if I copy it before, then each copy will create its own underlying list?

Mutable structs are very bad, especially if they behave like reference types in some aspects, but not in others. To avoid this issue, the default value should be a read-only empty list - so the struct itself will be immutable, only the underlying list (if any) can be modified. That would make the struct act just like a non-nullable reference type on the outside. You'll get an exception when adding to an uninitialized struct, but it still solves the problem of null references for pure read access (which tend to be much more common than write access).

Daniel Grunwald

To elaborate on why such a mutable struct is wrong:

Replace your private static field with a static automatic property, and your example program will print 0. (as it mutates the properties' return value, not the actual underlying field)

This is a massive pitfall! A NotSupportedException (uninitialized Collection is readonly) would be much better.

Dmitry

Why is it a problem to call the default constructor?

Andres

WTF!

Scooletz

This code is just wrong. I can imagine a plenty of people reading it and then trying to figure out what's going on. After 15 minutes they would go to the summary of the Collection class, oops! struct and read "Because it allows you to write this sort of code:". Then a facepalm would occur.

Ayende Rahien

Daniel, Actually, no, it would still work. The internal list is mutated, not the actual struct.

Daniel Grunwald

@Ayende Try it, I'm fairly sure your example program will break with an automatic property instead of a field. The actual struct is mutated when the internal list is created on the first Add() call. Your struct behaves like a value type before initialization, and like a reference type after. With an automatic property, there's no way to initialize the underlying field; only the properties' return value gets initialized - so in effect every access to the property returns a new collection.

Andres

April fools?

SergeyT.

@Daniel, @Ayende: actually there is even more subtle way to break this code.

Try to add readonly modifier to the "nums" field and run this sample one more time.

Karep

WTF? What i don't understand. What did you get by writing this class? That that you don't have to 'new' Collection?

Gerke Geurts

Why not be upfront about being immutable and use the following signature instead: public Collection Add(T other)?

Clay Ver Valen

You'll have to ensure inner has been initialized before every public method and get_*. Not that you can't, but not very elegant either.

Also, make sure to NOT to initialize it to a new instance of "Collection" or you'll get a StackOverflowException. Instead fully qualify it (e.g. System.Collections.ObjectModel.Collection() ) or use List, etc.

Fabian Schmied

@Ayende: I don't understand the purpose, even after your explanation. Are you saying you did this because you needed to define a static field holding an initialized list, but couldn't use a field initializer?

If so, why couldn't you use a field initializer?

Ayende Rahien

Fabian, The idea is that you have a "never null" collection.

Don't think just about this scenario, think about how useful it is when you KNOW that a collection can never be null.

Neil Mosafi

But the inner list can be null (and will be) so you gain very little apart from a bit of added indirection and possibly a source of some bugs (every property and method will have to lazy initialise the inner list or something similar). Better to initialise a private readonly field if you want to guarantee non nullness.

Ayende Rahien

Neil, The inner prop will be null, sure. But I can do the lazy init in a single class, not in all places.

Neil Mosafi

Sure, but I don't think the point of this post is to discuss building a lazy wrapper for a list. I will wait and see cos I'm not sure what it's about now!

Comment preview

Comments have been closed on this topic.

FUTURE POSTS

No future posts left, oh my!

RECENT SERIES

  1. Production postmortem (5):
    29 Jul 2015 - The evil licensing code
  2. Career planning (6):
    24 Jul 2015 - The immortal choices aren't
  3. API Design (7):
    20 Jul 2015 - We’ll let the users sort it out
  4. What is new in RavenDB 3.5 (3):
    15 Jul 2015 - Exploring data in the dark
  5. The RavenDB Comic Strip (3):
    28 May 2015 - Part III – High availability & sleeping soundly
View all series

Syndication

Main feed Feed Stats
Comments feed   Comments Feed Stats