How do you fix the wrong-case-sensitivity dictionary setting bug-pattern?

Question

There is a coding anti-pattern I've noticed (while using .Net). You declare a data class, which is supposed to have a dictionary field (or get/set Property), and lets call it 'Properties', for the sake of example. It has a string key type.

    public System.Collections.Generic.Dictionary<string, object> Properties { get; set; }

We might want to use it like a plain old data class. So, we can deserialize JSON or SQL rows or whatever, to fill in the properties dictionary. By seemingly good luck, our JSON serializer understands the dictionary type and can do that pretty well. BUT (here comes the problem...) in our business logic, we want this dictionary to be case-insensitive with how it treats keys.

"OK, no problem," you say. "Dictionaries can do that. Every time we write code that initializes a dictionary on the class, we will just do this:

    Properties = new Dictionary<string, object>(StringComparer.InvariantCultureIgnoreCase)

Well, do you see the problem with this? Because we put a public setter on there. Anyone can call this dictionary setter and put a case-sensitive dictionary onto our object instead. And Murphy assures us they will; not out of bad intentions, mind you, but because it is just all too easy to forget the optional StringComparer parameter to Dictionary.

And even worse, we have code that we don't own, in our JSON deserializer, or object mapping framework, which doesn't bother to use a StringComparer. Or perhaps it just chooses the wrong one, for instance StringComparer.CurrentCultureIgnoreCase, or OrdinalIgnoreCase. THis is clearly a disaster waiting to happen. What should we do about this?

Answer 1

Well, do you see the problem with this? Because we put a public setter on there.

So don't have a public setter.

Create a new class CaseInsensitiveDictionary that contains a private Dictionary initialized with the case-insensitivity. Only expose that private Dictionary via methods to interact with it (like add, remove, etc) - or only provide get access to it. You'll probably want to implement IEnumerable, IDictionary, and other interfaces.

public sealed class CaseInsensitiveDictionary<string,TValue>
{
    private Dictionary<string, TValue> _dictionary = new Dictionary<string, object>(StringComparer.InvariantCultureIgnoreCase);
   // ... constructors, add, remove, etc methods
}

Wherever you use the Dictionary now, replace it with this new class. If you need to handle JSON deserialization, etc you can provide methods to do that with the new class.

This is similar to what is done in the .NET framework - Dictionary vs SortedList for instance - a new class is created when a constraint (ordering) is needed.

Answer 2

mmathis's answer gives you the practical solution to the problem, but I'll look at this from a more theoretical point of view.

What you're looking at here is a very close corollary of the Liskov Substitution Principle - by declaring your member as a Dictionary, you are implicitly saying "any object which meets Dictionary's contract can go here". However, that's not what you actually want in this case as you are happy only with objects which meet your requirement of case-insensitivity.

This isn't quite the LSP as normally stated because we're not talking about substituting a subclass for a parent class, but it's pretty much the same thing as you have parameterised behaviour for your object. mmathis's answer solves that by restricting the contract.

Answer 3

A option I used in the past, as an alternative to an CaseInsensitiveDictionary was a CaseInsensitiveString type.

    public readonly struct CaseInsensitiveString : IEquatable<CaseInsensitiveString>, IComparable<CaseInsensitiveString>
    {
        public string Original { get; }

        public CaseInsensitiveString(string value)
        {
            Original = value ?? throw new ArgumentNullException(nameof(value));
        }

        public bool Equals(CaseInsensitiveString other) => other.Original.Equals(Original, StringComparison.InvariantCultureIgnoreCase);
        public override bool Equals(object? obj) => obj is CaseInsensitiveString cis && Equals(cis);
        public override int GetHashCode() => Original.GetHashCode(StringComparison.InvariantCultureIgnoreCase);
        public override string ToString() => Original;

       ...
    }

An just use this type as the Directory-Key. This approach works good if you use the Dictionary-Keys in other places as well. So all method arguments/class members that should be case-insenstive are automatically self-documenting and perform the correct comparisons.

Answer 4

A shortcut to not have to implement everything you can just derive:

public interface ICaseInsensitiveDictionary<T> : IDictionary<string, T>
{
}

public class CaseInsensitiveDictionary<T> : Dictionary<string, T>, ICaseInsensitiveDictionary<T>
{
    public CaseInsensitiveDictionary() : base(StringComparer.OrdinalIgnoreCase)
    {
    }
}

And then use it to declare properties:

public ICaseInsensitiveDictionary<object> Properties { get; set; }

or:

public CaseInsensitiveDictionary<object> Properties { get; set; }

Answer 5

This is one of the risks of taking the "Quick and Easy Path" of Automatic Properties.

if you need to control what comes in, then write your own setter method and copy the values of the given Dictionary object into your internal one, throwing an Exception if it contains two values that differ only by case.
Actually, you probably ought to be doing this anyway or your object could be "upset" by the given Dictionary being changed after you've taken "possession" of it.