Reputation: 10397
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
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