csharp

2 posts

Keep Your Collection Setters Private

When exposing properties of a class, you may find yourself immediately exposing a public getter and a public setter. This is a very common approach, especially in anemic models. However, this approach can be problematic when applied to collections. Allowing consumers of your collection to freely modify the entire collection is very destructive. In this post, I will go over two scenarios in which a public setter on your collection can get you into trouble.

You cannot iterate over a null collection

Lets compare two approaches to empty a collection. One approach that we could take is to call the .Clear() method like so:

myCollection.Clear();  

Another approach, assuming we had a public setter, could be done by setting the collection to null:

myCollection = null;  

This is the first problem area for allowing public setters on your collections. By allowing your consumers to directly modify the collection, it's no longer possible to control how they will interact with your collection. Setting the entire collection to null for example, can potentially cause problems later in your programs life cycle.

Iterating over your collection is one of these pain points. The following code will behave differently if the collection is set to null, or if it is simply empty.

for(var item in collection)  
{
    // do stuff with collection items
}

If the collection was set to null, you will run into a lovely NullReferenceException, like the one below.

An unhandled exception of type 'System.NullReferenceException' occurred in YourProgramHere.exe

This is because all collections expose a GetEnumerator() method, which foreach leverages to iterate over your collection. If the collection is null, its GetEnumerator() method cannot be called. However, if the collection was simply empty via .Clear(), the run time error would not occur.

Events will not be triggered

Another problem that can arise is that it be very hard, if not impossible, to know when your collection has changed. Consider the following MyCollection class.

public class MyCollection  
{
    public ObservableCollection<int> Numbers { get; set; }

    public MyCollection()
    {
        Numbers = new ObservableCollection<int>();
        Numbers.CollectionChanged += collectionChanged;
    }

    private void collectionChanged(object sender, NotifyCollectionChangedEventArgs args)
    {
        Console.WriteLine("Hey! Listen!");
    }
}

This class defines a collection that will output some text when the collection changes. Now while this example only shows one collection, it could be entirely possible that this collection has many subscribers that wish to be notified when its contents change.

Using this collection, the only time the collectionChanged event will fire is when you leverage its .Add(), .Remove(), etc methods. Setting the collection to null will indeed empty the collection, but its subscribers will be unaware of anything that has happened.

In summary, when you expose a public setter on a collection, you are exposing much more behavior than you need to. It gives a lot of unnecessary control to your consumers, which could potentially put your collection into an undesirable state.

Prefer readonly to const

I was recently reading Effective C# by Bill Wagnar. In the book, the author makes the statement that we should prefer readonly to constants. I wasn't immediately able to piece together why we should, I mean, what's even the difference between the two? To answer this, let's first discuss how they're the same.

Both are static

Now obviously static readonly is static because it is decorated by the static keyword. One thing you may have overlooked however though, is that the const keyword is implicitly declared as static.

Static, put simply, means that whatever member it is decorating, belongs to the class itself rather than the object. The static member must be referenced by type and not by an instance. For example, the following code would not compile:

public class TheClass  
{
    public static void TotallyStaticMethod() { ... }
}
var someClass = new TheClass();  
someClass.TotallyStaticMethod(); // cannot be accessed with an instance reference;  

The above would generate a compiler error, specifically: Member 'TheClass.TotallyStaticMethod()' cannot be accessed with an instance reference; qualify it with a type name instead.

Both are immutable

Once declared readonly or const, the value cannot change. Now, there is a slight difference as to when the values become truly immutable. The value of a const must be initialized when it is declared. No sooner (though that'd be impressive) and no later.

On the other hand, readonly may be initialized during its declaration or in the constructor of the class that it was declared. This is useful for facilitating dependency injection through the constructor or even configuration values.

The biggest difference between the two?

Const is evaluated at compile time

What does this mean exactly? When you set the value of a constant, the compiler will actually take your variable assignment and bake it directly into the IL.

To see this in action, let's say we have the following code snippet.

namespace AnotherAssembly  
{
    public class Library
    {
        public static readonly string ReadOnlyValue = "first readonly";
        public const string ConstValue = "first const"; 
    }
}

If you were to compile the above code, reference the DLL in another project, and inspect the DLL with ILSPY (or your disassembler of choice) you would see the following output:

This clearly shows that our readonly variable is a reference type, referencing the ReadOnlyValue in the Library class which lives in AnotherAssembly. On the other hand, the const variable is being directly loaded with the value "first const".

This means that if you were to change the value of a const in an assembly, the assemblies that depend on that const will have the old value until they are rebuilt. This could cause a lot of headaches down the road, which is why it's always best to reserve const for values that you know will never change.

So while it is true that const will be slightly more performant than readonly, it's going to be a negligible amount. Always keep in mind that premature optimization is the root of all evil!

Summary

  1. const is evaluated at compile time, readonly is evaluated at runtime.
  2. Prefer const for values that you know will never change, for any reason.
  3. When in doubt, use readonly (the performance gain is negligible).