-2
\$\begingroup\$

I've seen a lot of code bases use Dictionaries with strings as their key. This can be redundant and expensive, so I'm trying to make a class that extends List but takes any kind of enum. Do you know how I could make this any better?

public class EnumList<TEnum,T> : List<T> where TEnum : struct, Enum
{
    /// <summary>
    /// Maps an enum definition to a list of the same length, this avoids manual casting
    /// </summary>
    /// <param name="list"></param>
    /// <exception cref="ArgumentOutOfRangeException">List must be same length as enum definition</exception>
    public EnumList(List<T> list) 
    {
        if(list.Count > enumLength)
        {
            throw new ArgumentOutOfRangeException("list is greater than enum definition");
        }
        else if (list.Count < enumLength)
        {
            throw new ArgumentOutOfRangeException("list is less than enum definition");
        }
        AddRange(list);
    }
    private int enumLength = Enum.GetNames(typeof(TEnum)).Length;

    public T this[TEnum index]
    {
        get => this[Unsafe.As<TEnum, int>(ref index)];                
        set => this[Unsafe.As<TEnum, int>(ref index)] = value;                
    }
}
\$\endgroup\$
4
  • \$\begingroup\$ what's wrong with Dictionary<Enum, TValue> ? it's unclear what are you trying to do, as from the code, it seems you're trying to initiate a list of values that supposed to be mapped to the actual enum values, similar to what Enum.GetNames or Enum.GetValues do. if that the case, then you can just do a wrapper class with a private cache, where you initiate those enums values at startup, and reuse that class to get those values from the underlying dictionary. \$\endgroup\$
    – iSR5
    Apr 6 at 1:17
  • \$\begingroup\$ ... extends List... -> C# guidelines say do not inherit from List<T>; instead, use Collection<T> \$\endgroup\$
    – radarbob
    Apr 7 at 7:50
  • \$\begingroup\$ Is this code part of a working program? Or are you asking a general question? If the latter you should post it on Stackexchange.softwareengineering; it does not meet the guidelines for Code Review. \$\endgroup\$
    – radarbob
    Apr 7 at 7:55
  • \$\begingroup\$ This SO entry looks like it may answer your question. \$\endgroup\$
    – radarbob
    Apr 7 at 7:59

1 Answer 1

3
\$\begingroup\$

It's unclear what problem you're trying to solve, and your implementation has issues.

I mostly agree that there's often a better pattern than using Dictionary<string, TValue> to map human-readable keys to value. Using a dictionary with an enum as the key type, however, should be fine and eliminate the use of magic string and possible overhead of computing string hashes: since C# enums have an integral numeric type as an underlying type (int by default), their hash code is equal to their value.

Now dictionaries might have slower access than lists (although you would need to provide performance measurements to make that statement), but is that a problem you encountered?

In my experience, Dictionary<string, TValue> is usually used when working with external resources, where using string as the key type avoids parsing enum values from strings received from file access or network requests. Your solution doesn't provide a solution for these kind of use case.

Now, your implementation has several issues that would keep me from actually using it:

IList<T> interface implementation

By inheriting from List<T>, you're implicitly implementing the IList<T> interface, including methods like Add and Remove.

You clearly don't want that, by constraining instantiation to only allow lists and enums with the same value count.

However, you can add values to an EnumList, but can't access them later, making the operation nothing more than a waste of resources. Worse, if you remove items from an EnumList, accessing some indices can cause an ArgumentOutOfRangeException.

The following code causes such a crash:

enum Foo { Bar, Baz }
static void Main(string[] args)
{
    var example = new EnumList<Foo, int>(new List<int> { 1, 2 });
    example.RemoveAt(0);
    Console.WriteLine(example[Foo.Baz]);
}

Your current implementation looks more like an IReadOnlyList. If that's really what you have in mind, make it explicit.

Enums can map arbitrary values

Enums can map arbitrary values to symbols, there is no guarantee that they will map to consecutive indices. Again, this leads to ArgumentOutOfRangeException, such as in the following code:

enum Foo { Bar = 2 }
static void Main(string[] args)
{
    var example = new EnumList<Foo, int>(new List<int> { 1 });
    Console.WriteLine(example[Foo.Bar]);
}

There are very legitimate use cases for enums with non-consecutive values. Error codes comes to mind as an example.

Also Flags enums are expected to be completely busted.

Excessive rigidity

You only allow instantiation by taking a List<T> as argument. This means I have to first: allocate a List<T>, then: allocate an EnumList. Instead, consider allowing any ICollection<T> to build an EnumList.

The requirement of having exactly the same count of values in the enum and the mapped list means that any change to the enum during development will cause all EnumList instantiation to fail and need rework.

\$\endgroup\$
0

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