10

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?

1

5 Answers 5

28

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.

8
  • 8
    "So don't have a public setter." This; there are very few times a public setter is the right answer (with a possible exception for POCOs). Mar 23, 2023 at 17:26
  • 6
    I'm not a .net person so I'm probably missing something here, but isn't this actually two separate solutions? Why do I need the custom class if the public setter is removed, can it not privately initialise the Properties field with a normal Dictionary with the desired comparer?
    – Bergi
    Mar 24, 2023 at 2:47
  • 2
    By the way, this pattern is called a Decorator.
    – Philipp
    Mar 24, 2023 at 8:51
  • 2
    @Bergi The type of properties being CaseInsensitiveDictionary tells me more than if the type is Dictionary.
    – Taemyr
    Mar 24, 2023 at 12:41
  • 2
    I think I would extend dictionary rather than write a wrapper in this case. If I wrote a wrapper I would definitely have it implement IDictionary.
    – Taemyr
    Mar 24, 2023 at 12:42
11

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.

8

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.

2

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; }
2
  • With an interface or an unsealed class couldn’t anyone remove the case-insensitive behaviour?
    – dlrlc
    Mar 28, 2023 at 19:22
  • In that case don't expose the setter. public CaseInsensitiveDictionary<object> Properties { get; } = new CaseInsensitiveDictionary<object>() Sealing will prevent creating derived classes. (commonly interfaces are used to be able to change the implementation so you might not need/want it).
    – Wouter
    Mar 28, 2023 at 19:28
-3

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.

Not the answer you're looking for? Browse other questions tagged or ask your own question.