API Proposal: Add a Generic version of GetValues to Enum (probably GetName/GetNames)

[andreise]

I suggest to implement Generic version of GetValues method in the Enum class.

In present, if we going to get an typed array of some enum values we have to write following code:
SomeEnum[] values = (SomeEnum[])Enum.GetValues(typeof(SomeEnum));

It seems as unconvenient way.
In my opinion, the following Generics-based syntax should seems shorter, more convenient and safely (by reason of type casting necessity):
SomeEnum[] values = Enum.GetValues<SomeEnum>();

The possible way to implement this syntax is:

public static TEnum[] GetValues<TEnum>() where TEnum : struct
{
    Type enumType = typeof(TEnum);
    return (TEnum[])enumType.GetEnumValues();
}

This proposal is inspired by the great Generic-based version of Enum.TryParse method laying besides with legacy non-Generic version:

public static bool TryParse(Type enumType, String value, out Object result)
public static bool TryParse<TEnum>(String value, out TEnum result) where TEnum : struct

If proposal to implement Generic version of GetValues will be accepted I guess following Generics-based implementations of GetName GetNames should be implemented for symmetry reason:

public static String GetName<TEnum>(Object value) where TEnum : struct
{
    Type enumType = typeof(TEnum);
    return enumType.GetEnumName(value);
}

public static String[] GetNames<TEnum>() where TEnum : struct
{
    Type enumType = typeof(TEnum);
    return enumType.GetEnumNames();
}

I introduce possible implementation of this proposal in dotnet/coreclr/pull#16557

This is a small point improvement suggestion.
It can be implemented separately of as a part of total Enum improvements discussing in dotnet/corefx/#15453

[terrajobst]

While @joperezr owns this are I believe this issue is read for an API review. @joperezr, feel free to unmark if you think otherwise 😄

[TylerBrinkley]

Split off from dotnet/corefx#15453

Enums are essential commonly used types but have several areas in need of improvement. For each non-generic Enum method there should be an equivalent generic version.

Rationale and Usage

1. Nearly all of Enum's static methods are non-generic leading to the following issues.
   • Requires boxing for methods with enum input parameters losing type-safety, eg. IsDefined and GetName.
   • Requires casting/unboxing for methods with an enum return value, eg. ToObject and GetValues.
   • Requires the enum type to be explicitly specified as an argument.
   • Requires invocation using static method syntax.

With this proposal implemented what used to be this to validate a standard enum value

MyEnum value = ???;
bool isValid = Enum.IsDefined(typeof(MyEnum), value);

now becomes this

MyEnum value = ???;
bool isValid = value.IsDefined();

With this implemented it will address dotnet/corefx#10692.

Proposed API

namespace System {
    public abstract class Enum : ValueType, IComparable, IConvertible, IFormattable {
        public static string Format<TEnum>(TEnum value, string format) where TEnum : struct, Enum;
        public static string GetName<TEnum>(this TEnum value) where TEnum : struct, Enum;
        public static IReadOnlyList<string> GetNames<TEnum>() where TEnum : struct, Enum;
        public static IReadOnlyList<TEnum> GetValues<TEnum>() where TEnum : struct, Enum;
        public static bool IsDefined<TEnum>(this TEnum value) where TEnum : struct, Enum;
        public static TEnum ToObject<TEnum>(object value) where TEnum : struct, Enum;
        public static TEnum ToObject<TEnum>(sbyte value) where TEnum : struct, Enum;
        public static TEnum ToObject<TEnum>(byte value) where TEnum : struct, Enum;
        public static TEnum ToObject<TEnum>(short value) where TEnum : struct, Enum;
        public static TEnum ToObject<TEnum>(ushort value) where TEnum : struct, Enum;
        public static TEnum ToObject<TEnum>(int value) where TEnum : struct, Enum;
        public static TEnum ToObject<TEnum>(uint value) where TEnum : struct, Enum;
        public static TEnum ToObject<TEnum>(long value) where TEnum : struct, Enum;
        public static TEnum ToObject<TEnum>(ulong value) where TEnum : struct, Enum;
    }
}

API Details

This proposal makes use of a C# language feature that needs to be added in order for this proposal to make the most impact.

This proposal specifies extension methods within System.Enum and as such requires C# to allow extension methods within non-static classes as is proposed in csharplang#301. Promoting these to extension methods later would be a breaking change due to csharplang#665 but I feel this is acceptable.

Alternatively, the extension methods could be defined in a separate static EnumExtensions class. This is uglier but would avoid this issue and the extension methods would be available immediately instead of needing to wait for a later C# version to support this.

Open Questions

• Is promoting the System.Enum extension methods later an acceptable breaking change due to csharplang#665? If not should we wait for a later C# version that supports extension methods in System.Enum or should we introduce a separate EnumExtensions class?

Updates

• Changed GetNames and GetValues to return an IReadOnlyList instead of IEnumerable.
• Added Format.

[terrajobst]

@TylerBrinkley

This looks like a separate proposal. Let's keep this focused on just this API:

namespace System
{
    public partial abstract class Enum
    {
       public static TEnum[] GetValues<TEnum>() where TEnum : Enum;
    }
}

This looks reasonable and we agree that it makes calling code nicer. @tannergooding brought up runtime concerns around polluting generic instantiations. @jkotas what are your thoughts on this?

[tannergooding]

Right, just want to confirm that there isn't any concern around adding a generic API to a non-generic type. IIRC, there was concerns raised around that in the past.

[TylerBrinkley]

This looks like a separate proposal.

I've moved those proposed API's back to dotnet/corefx#15453.

I'm especially opposed to returning an array for GetValues as an array cannot be reused and must be allocated on each call. It should instead return an IReadOnlyList<TEnum> as that could then be shared even if the current implementation doesn't do so.

[jkotas]

@jkotas what are your thoughts on this?

We do have a number of non-generic methods in the surface. How are we deciding when it is worth creating duplicate generic wrapper to save a bit of typing, e.g. why not have generic versions of Enum.GetName, Enum.IsDefined or Attribute.GetCustomAttributes too?

If Enum.GetValues is different enough from the other APIs to deserve the generic wrapper, I do not have a problem with adding it.

returning an array for GetValues as an array cannot be reused

Does it really matter? Enum.GetValues is not the kind of API that is likely to be on a hot-path, and returning interface instead of array would add a different kind of overhead.

[TylerBrinkley]

Does it really matter? Enum.GetValues is not the kind of API that is likely to be on a hot-path, and returning interface instead of array would add a different kind of overhead.

It's common to use enums to represent options in a dropdown and if GetValues returns an array then each time a page is requested with that dropdown it will add non-insignificant allocations, especially when there are many options.

[tannergooding]

it will add non-insignificant allocations,

It will be a single allocation (the array) that is relatively small overall (even for the full country list, you are looking at under 2kb for the array, if you assume ~200 countries and 8 bytes per entry).

[tannergooding]

The cost of the allocation could also be amortized by caching on the UX side or be utilizing an array pool.

[TylerBrinkley]

The cost of the allocation could also be amortized by caching on the UX side or be utilizing an array pool.

That's true. If a server were to do the caching that would reduce the allocation cost.

I'd still prefer it to return an IReadOnlyList<TEnum> but acknowledge returning an array would be acceptable.

[svick]

@tannergooding How do you use an array pool here, when the implementation of GetValues creates the array, but only the caller knows when it's no longer necessary?

[TylerBrinkley]

FWIW, from my benchmarking in dotnet/coreclr#22161 the current implementation of the non-generic version of this method allocates much more than would be expected, about 1kb for an enum with 36 members.

[GSPP]

The lack of convenient enum reflection APIs has been long-standing. At a minimum, there should be a maximally convenient API for all common enum reflection scenarios.

If such a convenient API does not have good performance it might make sense to add a high-performance API in some form.

But callers can already ensure optimal performance by doing caching work themselves.

IMO, the most important use case, that is not currently served, is a convenient API.

I have seen many enum tickets on this issue tracker. I think it would be best if all of them were looked at together and a holistic solution was implemented.

[terrajobst]

Video

I stand corrected, we believe we'd prefer this API shape:

namespace System
{
    public partial abstract class Enum
    {
        public static TEnum[] GetValues<TEnum>() where TEnum : struct, Enum;
        public static string GetName<TEnum>(TEnum value) where TEnum : struct, Enum;
        public static string[] GetNames<TEnum>() where TEnum : struct, Enum;     
        public static bool IsDefined<TEnum>(TEnum value) where TEnum : struct, Enum;       
        public static bool HasFlag<TEnum>(TEnum value, TEnum test) where TEnum : struct, Enum;
    }
}

[TylerBrinkley]

Please consider adding HasAnyFlags and HasAllFlags as opposed to HasFlag which is confusing whether it checks if the value has all of the flags defined in the other parameter or any of the flags in the other parameter.

[terrajobst]

Hmm, not sure it's worth it. HasFlag today means HasAllFlags (runtime implemented). When would you ever want the semantics of HasAnyFlags()?

[TylerBrinkley]

Granted, most users will only be checking one enum flag value and as such the semantics between HasAllFlags and HasAnyFlags are identical.

However, consider a DaysOfWeek enum specified as such.

[Flags]
public enum DaysOfWeek
{
    None = 0,
    Sunday = 1,
    Monday = 2,
    Tuesday = 4,
    Wednesday = 8,
    Thursday = 16,
    Friday = 32,
    Weekdays = Monday | Tuesday | Wednesday | Thursday | Friday,
    Saturday = 64,
    Weekend = Sunday | Saturday
}

If you wanted to determine if a given DaysOfWeek value contains a weekday your code would look like this

(value.HasFlag(DaysOfWeek.Monday) || value.HasFlag(DaysOfWeek.Tuesday) || value.HasFlag(DaysOfWeek.Wednesday) || value.HasFlag(DaysOfWeek.Thursday) || value.HasFlag(DaysOfWeek.Friday))

or this without HasAnyFlags.

(value & DaysOfWeek.WeekDays) != 0

I don't think this example is too far-fetched and can definitely see this being used on something like AttributeTargets, BindingFlags, or a flag enum representing permissions such as Admin, Manager, User, Developer, etc.

[jkotas]

(value & DaysOfWeek.WeekDays) != 0

IMHO, this is the best way to check for presence of any flag: There is no ambiguity about what it does, and it is more concise and faster than Enum.HasFlags. Is there really enough usability value in wrapping this in a helper method? If you are working with flags, you have to be familiar with how bit masks work and checking the bits using the built-in C# operators should come pretty natural.

[TylerBrinkley]

You could use that same argument for not adding HasFlag. However, I suppose the most likely reason it was added was for the common case of checking for a single flag as the name implies. My vote, FWIW, is to have both less ambiguously named methods.

Could we consider any other flag enum operations as proposed in dotnet/corefx#34079? The one I consider to have the most value in that proposal is the GetFlags method which returns an IEnumerable<T> of the individual flags that compose a flag enum value which could be very useful but is non-trivial to write even for a specific enum type.

Also, if we ever want to make GetName, IsDefined, or HasFlag extension methods in the future if/when C# supports declaring extension methods in a non-static class such as Enum we would break user code if they use a using static directive due to dotnet/csharplang#665. Consider this code which would break.

using System;
using static System.Enum;

namespace ConsoleApplication1
{
    public static class Program
    {
        public static void Main()
        {
            // IsDefined can not be found if it's promoted to an extension method as using
            // static requires extension methods to be invoked with extension method syntax
            Console.WriteLine(IsDefined(DayOfWeek.Wednesday)); 
        }
    }
}

I think using static directive usage is pretty low and thus would support that breaking change.

[jkotas]

To make progress, it may be best to scope this issue to just GetValues, GetName, GetNames and leave the discussion about helper methods for flags operations to dotnet/corefx#34079.

[TylerBrinkley]

Fair enough. My other point about the extension method issue should be discussed however.

[terrajobst]

Fair enough. My other point about the extension method issue should be discussed however.

C# doesn't allow extension methods inside of non-static classes. This would require a new type (like System.EnumExtensions). Given that, I don't believe this minor syntactic sugar is worth it.

[terrajobst]

To make progress, it may be best to scope this issue to just GetValues, GetName, GetNames and leave the discussion about helper methods for flags operations to dotnet/corefx#34079.

We should include HasFlag for internal consistency. For better or worse, Enum included this API and adding the generic alternative is easier to explain than leaving it off. It also means developers don't have to repeat themselves:

var x = Enum.HasFlag(dayOfWeek, DaysOfWeek.Monday);
var y = (dayOfWeek & DaysOfWeek.Monday) == DaysOfWeek.Monday;

However, I really don't think HasAnyFlag is worth it. For starters, it's much more rare and second the C# syntax is even shorter:

var x = Enum.HasAnyFlag(dayOfWeek, DaysOfWeek.Workdays);
var y = (dayOfWeek & DaysOfWeek.Workdays) != 0;

[jkotas]

We should include HasFlag for internal consistency.

@terrajobst The generic HasFlag does not provide internal consistency. The generic HasFlag is static method, but the non-generic method is a instance method that is more convenient to use.

var x = Enum.HasFlag(dayOfWeek, DaysOfWeek.Monday);

You can write this today as more concise var x = dayOfWeek.HasFlag(DaysOfWeek.Monday) and it works just fine. We run complex pattern matching in the JIT to convert it back to the efficient code that you would get from (dayOfWeek & DaysOfWeek.Monday) == DaysOfWeek.Monday;

The syntax sugar provided by the generic API is longer. I do not see who would ever want to use this longer version in ordinary code. The only difference is going to be that the pattern matching in the JIT for this is going to need to be 50% more complex so that the generic API is also converted back to the efficient code.

Enum.HasFlag would only make sense if we had a complete set of generic operations for flags algebra, but that should be better discussed separately.

I really don't think HasAnyFlag is worth it. For starters, it's much more rare and second the C# syntax is even shorter:

Checking for all flags in the set is rare. The much more frequent flag operations are to check for exact one flag to be set or to check for any flag in a set to be set. As you have said the C# syntax for that is shorter. Everybody should better be using that in the first place to save on the busy work required to make the Enum.HasFlag work efficiently.

[terrajobst]

@terrajobst The generic HasFlag does not provide internal consistency. The generic HasFlag is static method, but the non-generic method is a instance method that is more convenient to use.

I see; HasFlag() is the only instance method in this set. In that case I agree that leaving it off makes more sense, which brings us back to:

namespace System
{
    public partial abstract class Enum
    {
        public static TEnum[] GetValues<TEnum>() where TEnum : struct, Enum;
        public static string GetName<TEnum>(TEnum value) where TEnum : struct, Enum;
        public static string[] GetNames<TEnum>() where TEnum : struct, Enum;     
        public static bool IsDefined<TEnum>(TEnum value) where TEnum : struct, Enum;       
    }
}

[TylerBrinkley]

I'd love to submit the PR for these additions but would like to wait until the repo's are merged before doing so if no one else objects.

[TylerBrinkley]

Also, with nullability now in C# 8, GetName should return string? as null is returned when the value is not defined.

[terrajobst]

I'd love to submit the PR for these additions but would like to wait until the repo's are merged before doing so if no one else objects.

No objections; this is going to be a 5.0 feature anyway.

[danmoseley]

@TylerBrinkley still interested?

[danmoseley]

re-adding approved label