Chris McCall
Chris McCall

Reputation: 10397

What are the drawbacks of this pattern to enforce immutability?

Given the following class:

public static class ComboModel
{
    public static Dictionary<int, int[]> Items()
    {
        return new Dictionary<int, int[]>
        {
            {
                1, 
                new[] { 
                    2,
                    3,
                    4  
                }
            }
        };
    }
}

I want to ensure immutability of the data in the class, but this feels javascript-y and has drawn some vague (but strong) criticism from my peers. What is wrong with it?

Upvotes: 1

Views: 121

Answers (1)

Evk
Evk

Reputation: 101473

Just use ReadOnlyDictionary, and allocate it only once:

public static class ComboModel {
    private static readonly ReadOnlyDictionary<int, int[]> _items = new ReadOnlyDictionary<int, int[]>(new Dictionary<int, int[]> {
        {
            1,
            new[] {
                2,
                3,
                4
            }
        }
    });

    public static IReadOnlyDictionary<int, int[]> Items
    {
        get { return _items; }
    }
}

Note that not only you allocate new instance on each call, as others already mentioned - you also provide wrong feeling to the caller that he can modify that dictionary. In ReadOnlyDictionary there are no methods that can modify it. There are other benefits when caller knows structure it received cannot be changed: for example he can safely process items there with multiple threads.

Update: of course readonly collections do not magically make objects stored in that collections readonly - just the collection itself. If you want to ensure immutability of int[] arrays in your example, just make them readonly too:

public static class ComboModel {
    private static readonly IReadOnlyDictionary<int, ReadOnlyCollection<int>> _items = new ReadOnlyDictionary<int, ReadOnlyCollection<int>>(new Dictionary<int, ReadOnlyCollection<int>> {
        {
            1,
            Array.AsReadOnly(new[] {
                2,
                3,
                4
            })
        }
    });

    public static IReadOnlyDictionary<int, ReadOnlyCollection<int>> Items
    {
        get { return _items; }
    }
}

Upvotes: 4

Related Questions