+972 52-548-6969
Explain this code: Answers
time to read 1 min | 170 words
The reason this code is useful?
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
The fact that it's missing an initializer for nums?
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.
@Richard, it will initialize all fields to their default value, which is null in the case of IList<T>, 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)
@Thomas: Maybe, Add method is initializing the inner field before use it.
@Mads, yes, this is what I had in mind ;)
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).
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.
Why is it a problem to call the default constructor?
WTF!
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.
Daniel, Actually, no, it would still work. The internal list is mutated, not the actual struct.
@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.
April fools?
@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.
WTF? What i don't understand. What did you get by writing this class? That that you don't have to 'new' Collection?
Why not be upfront about being immutable and use the following signature instead: public Collection<T> Add(T other)?
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<T>() ) or use List<T>, etc.
@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?
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.
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.
Neil, The inner prop will be null, sure. But I can do the lazy init in a single class, not in all places.
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