2
\$\begingroup\$

I created these little utility methods to build fluent C# dictionaries. The value declarations shall be quick to write and easy to comprehend when somebody views the code. Usage conditions are as follows:

  • many keys point to the same values
  • group keys pointing to one value. Declare value only once for better visual comprehension.
  • only relatively few values (usually 10 to 20, rarely up to 100)
  • main application purpose: replace switch-case blocks in legacy code

The keys/values can also be added otherwise, such as from all constants or Resource content, and can be of any type useful to be declared in code.

In addition, I created an extension convenience method: simply .ToDictionary(), instead of constructor lambdas kv => kv.Key, kv => kv.Value

Is this good, or are there any problems or better ideas? What about performance when used frequently, compared to switch-case?

Usage/test:

class Program
{
    public const string RoadVehicles = "RoadVehicles";
    public const string RailVehicles = "RailVehicles";
    public const string Watercraft = "Watercraft";
    public const string Aircraft = "Aircraft";

    public static readonly IReadOnlyDictionary<string, string> GroupsForVehicles =
        FluentDictionaries.KeysToValue(
            RoadVehicles, "Car", "Truck", "Tractor", "Motorcycle")
        .KeysToValue(
            RailVehicles, "Locomotive", "Railcar", "Powercar", "Handcar")
        .KeysToValue(
            Watercraft, "Ship", "Sailboat", "Rowboat", "Submarine")
        .KeysToValue(
            Aircraft, "Jetplane", "Propellerplane", "Helicopter", "Glider", "Balloon")
        .ToDictionary();


    static void Main(string[] args)
    {
        foreach (var key in GroupsForVehicles.Keys.OrderBy(key => key))
        {
            Console.WriteLine(key + ": " + GroupsForVehicles[key]);
        }
        Console.ReadLine();
    }
}

The fluent methods:

public static class FluentDictionaries
{
    public static IEnumerable<KeyValuePair<TKey, TValue>> KeysToValue<TKey, TValue>(TValue value, params TKey[] keys)
    {
        return keys.Select(key =>
            new KeyValuePair<TKey, TValue>(key, value));
    }

    public static IEnumerable<KeyValuePair<TKey, TValue>> KeysToValue<TKey, TValue>(
        this IEnumerable<KeyValuePair<TKey, TValue>> previous, TValue value, params TKey[] keys)
    {
        return previous.Concat(keys.Select(key => 
            new KeyValuePair<TKey, TValue>(key, value)));
    }

    public static Dictionary<TKey, TValue> ToDictionary<TKey, TValue>(this IEnumerable<KeyValuePair<TKey, TValue>> keyValuePairs)
    {
        return keyValuePairs.ToDictionary(kv => kv.Key, kv => kv.Value);
    }
}

Test output of the program above:

Balloon: Aircraft

Car: RoadVehicles

Glider: Aircraft

Handcar: RailVehicles

Helicopter: Aircraft

Jetplane: Aircraft

Locomotive: RailVehicles

Motorcycle: RoadVehicles

Powercar: RailVehicles

Propellerplane: Aircraft

Railcar: RailVehicles

Rowboat: Watercraft

Sailboat: Watercraft

Ship: Watercraft

Submarine: Watercraft

Tractor: RoadVehicles

Truck: RoadVehicles
\$\endgroup\$
  • \$\begingroup\$ What is your class doing which the regular Dictionary cannot do? \$\endgroup\$ – CodingYoshi Jan 29 '17 at 0:46
  • \$\begingroup\$ It's basically convenience, and to make a static key-value mapping in code visually better and reduce possible mistakes due to repeating the wrong result value (using common Dictionary initializer). No additional functionality. \$\endgroup\$ – Erik Hart Jan 29 '17 at 1:13
4
\$\begingroup\$

This sounds like a good idea but I don't think it is, at least not with the current method names that I agree with @denis are very confusing.

The name KeysToValue should actually be AddValueWithKeys because this is the order of parameters and it better suggests that the first parameter is a value and not a key. KeysToValue sounds like a query that gets something or converts keys to values etc.


I also find the extension should extend the dictionary and not some arbitrary IEnumerable<KeyValuePair<TKey, TValue>> because if it will throw a duplicate key exception, it will be hard to find where it happened and it's of more use if extending a dictionary. If it however extends the IEnumerable then the name should be Concat.

The improved version could look like this:

public static Dictionary<TKey, TValue> AddValueWithKeys<TKey, TValue>(
    this Dictionary<TKey, TValue> dictionary, 
    TValue value, 
    params TKey[] keys)
{
    foreach (var key in keys) dictionary.Add(key, value);
    return dictionary;
}

and is much easier to use as it now requries only a single extension

public static readonly IReadOnlyDictionary<string, string> GroupsForVehicles =
    new Dictionary<string, string>()
        .AddValueWithKeys(RoadVehicles, "Car", "Truck", "Tractor", "Motorcycle");
\$\endgroup\$
  • \$\begingroup\$ Adding key-value pairs to the dictionary may be a good solution, instead of IEnumerable<KeyValuePair<,>>.ToDictionary(). I just wanted to get rid of the mandatory, additional generic type declaration. I did not find the Dictionary constructor taking an IEnumerable<KeyValuePair<,>> as argument, only IDictionary<,>. Also, LINQ .ToDictionary(...) only supports it when specifying the key/value lambdas (therefore my convenience method). \$\endgroup\$ – Erik Hart Jan 29 '17 at 13:39
  • \$\begingroup\$ @ErikHart oh, my bad, I was pretty sure there was such overload. I'll remove this part from my answer. You're right, it takes a dictionary. \$\endgroup\$ – t3chb0t Jan 29 '17 at 13:47
3
\$\begingroup\$

Your Dictionary<,> instantiation doesn't looks fluent to me at all, instead it looks overly-coupled with all kinds of extension method calls and it's kinda counter intuitive as the first element in a KeyValuePair<,> is usually the key, but in your case it's the value/s.

It's also hard to tell if all of those "Car", "Truck", "Tractor", "Motorcycle" will be connected in a way to your key or wait.. those were the keys..

I would suggest to create a separate class, which cleans up that for you:

public class FluentDictionary<TKey, TValue> : IDictionary<TKey, TValue>
{
    private readonly Dictionary<TKey, TValue> _dictionary;

    public ICollection<TKey> Keys => _dictionary.Keys;
    public ICollection<TValue> Values => _dictionary.Values;

    public int Count => _dictionary.Count;
    public bool IsReadOnly => false;

    public TValue this[TKey key]
    {
        get { return _dictionary[key]; }
        set { _dictionary[key] = value; }
    }

    public FluentDictionary()
    {
        _dictionary = new Dictionary<TKey, TValue>();
    }

    public FluentDictionary(Dictionary<TKey, TValue> source)
    {
        _dictionary = source;
    }

    public FluentDictionary(IEnumerable<KeyValuePair<TKey, TValue>> source)
    {
        _dictionary = source.ToDictionary(pair => pair.Key, pair => pair.Value);
    }

    IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();

    public IEnumerator<KeyValuePair<TKey, TValue>> GetEnumerator() => _dictionary.GetEnumerator();

    public void Add(TKey key, TValue value) => _dictionary.Add(key, value);

    public void Add(KeyValuePair<TKey, TValue> item) => Add(item.Key, item.Value);

    public void Add(TKey[] keys, TValue value)
    {
        foreach (var key in keys)
        {
            Add(key, value);
        }
    }

    public void Clear() => _dictionary.Clear();

    bool ICollection<KeyValuePair<TKey, TValue>>.Contains(KeyValuePair<TKey, TValue> item)
    {
        TValue value;
        return _dictionary.TryGetValue(item.Key, out value) &&
                EqualityComparer<TValue>.Default.Equals(value, item.Value);
    }

    void ICollection<KeyValuePair<TKey, TValue>>.CopyTo(KeyValuePair<TKey, TValue>[] array, int arrayIndex)
    {
        ICollection<KeyValuePair<TKey, TValue>> collection = new List<KeyValuePair<TKey, TValue>>(_dictionary);
        collection.CopyTo(array, arrayIndex);
    }

    bool ICollection<KeyValuePair<TKey, TValue>>.Remove(KeyValuePair<TKey, TValue> item)
    {
        return this.Contains(item) && Remove(item.Key);
    }

    public bool ContainsKey(TKey key) => _dictionary.ContainsKey(key);

    public bool Remove(TKey key) => _dictionary.Remove(key);

    public bool TryGetValue(TKey key, out TValue value) => _dictionary.TryGetValue(key, out value);
}

Some extra functionality you might want to have is a way of obtaining all of the keys that point to specific value:

public TKey[] FindKeysMatchingValue(TValue value) => _dictionary.Where(
        pair => EqualityComparer<TValue>.Default.Equals(pair.Value, value))
    .Select(pair => pair.Key)
    .ToArray();
\$\endgroup\$
  • \$\begingroup\$ Nicely done! ;) \$\endgroup\$ – 404 Jan 30 '17 at 17:01
2
\$\begingroup\$

I like your idea, but instead of ending the sequence of KeysToValue() calls with a call to ToDictionary(), I would make an Add-extension to IDictionary like below. You then only need one extension, and it is more flexible:

  public static class FluentDictionaries
  {
    public static IDictionary<TKey, TValue> Add<TKey, TValue>(this IDictionary<TKey, TValue> dict, TValue value, params TKey[] keys)
    {
      Array.ForEach(keys, k => dict.Add(k, value));
      return dict;
    }

    public static IReadOnlyDictionary<TKey, TValue> AsReadOnly<TKey, TValue>(this IDictionary<TKey, TValue> dict)
    {
      return dict as IReadOnlyDictionary<TKey, TValue>;
    }

  }

Use case:

static void Main(string[] args)
{
  string RoadVehicles = "RoadVehicles";
  string RailVehicles = "RailVehicles";
  string Watercraft = "Watercraft";
  string Aircraft = "Aircraft";

  IReadOnlyDictionary<string, string> GroupsForVehicles = new Dictionary<string, string>()
      .Add(RoadVehicles, "Car", "Truck", "Tractor", "Motorcycle")
      .Add(RailVehicles, "Locomotive", "Railcar", "Powercar", "Handcar")
      .Add(Watercraft, "Ship", "Sailboat", "Rowboat", "Submarine")
      .Add(Aircraft, "Jetplane", "Propellerplane", "Helicopter", "Glider", "Balloon")
      .AsReadOnly();

  foreach (var key in GroupsForVehicles.Keys.OrderBy(key => key))
  {
    Console.WriteLine(key + ": " + GroupsForVehicles[key]);
  }
}
\$\endgroup\$
  • \$\begingroup\$ Thanks, but naming them just Add(...) would confuse users, because of the reversed key-value order. This is necessary, because the params keyword can be applied only to the last method parameter (also, using a single array for both key and values would not be good: 1. possibly different types, 2. even more confusion what is key or value). The AsReadOnly() extension will not help me much, because I mostly apply this to static readonly fields, where I have to declare the type anyway. \$\endgroup\$ – Erik Hart Jan 29 '17 at 13:51
  • \$\begingroup\$ @ErikHart: I really don't understand your arguments. Feel free to call the method whatever you want :-). The AsReadOnly() was just ment to be a "Nice-to-have" method which by the way works perfectly with static readonly fields. \$\endgroup\$ – Henrik Hansen Jan 29 '17 at 14:10

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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