5
\$\begingroup\$

The idea is to perform a bit-wise scan of an ushort provided to find any and all matches within an enum collection.

The code I developed is as follows:

public List<T> GetEnumItemsFromUshort<T>(ushort input) where T : struct, IComparable, IConvertible, IFormattable
    {
        var output = new List<T>();
        foreach (T enumValue in Enum.GetValues(typeof(T)))
        {
            if ((input & enumValue.ToUInt16(new CultureInfo("en-US"))) == enumValue.ToUInt16(new CultureInfo("en-US")))
            {
                output.Add(enumValue);
            }
        }
        return output;
    }

As you can see, I use a generic type to define the return type.

I searched the site and found that constraining the generic type with the following struct, IComparable, IConvertible, IFormattable, makes it behave like an enum.

The code generates a list of the enum provided to contains the matches found.

Then it proceeds to scan the input value to find any and all items that masked (a bit-wise operation as stated before) do match.

If a match is found it adds the enum case to the return list.

I want to know whether this is a good or bad implementation and ways to make it better, it performs as expected but I feel like there's room for optimization.

To provide an example, suppose the following enum and input:

public ushort sample = 6;

public enum SampleCases
{
    Case1 = 0x1,
    Case2 = 0x2,
    Case3 = 0x4,        
    Case4 = 0x8,
}

The output for this code would be a List of type SampleCases with 2 items, Case2 and Case 3 since both (bit-wise) are contained in the sample provided. Tell me if this works as an example for you.

\$\endgroup\$
  • \$\begingroup\$ Can you post an example with the desired output? It sounds to me as you're trying to implement SomeEnum x = (SomeEnum)input; \$\endgroup\$ – Lasse Vågsæther Karlsen Nov 17 '15 at 11:56
  • \$\begingroup\$ I posted an edit, tell me whether this works for you as an example \$\endgroup\$ – Oscar Guillamon Nov 17 '15 at 12:04
  • \$\begingroup\$ What do you intend to use this list for? \$\endgroup\$ – Lasse Vågsæther Karlsen Nov 17 '15 at 12:07
  • \$\begingroup\$ I intend to use this list to retrieve any and all buttons pressed on a device, since it returns the information in a bit-wise fashion \$\endgroup\$ – Oscar Guillamon Nov 17 '15 at 12:10
7
\$\begingroup\$

Code can be greatly simplified but first I'd list few things:

1) If your enum is a bitmap then you should mark it with [Flags].

[Flags]
public enum SampleCases
{
    Case1 = 0x1,
    Case2 = 0x2,
    Case3 = 0x4,        
    Case4 = 0x8,
}

2) you shouldn't convert an enum value to ushort, enums default type is int and for [Flags] you may (easily?) run out of bits. If value is out of ushort range (your input) simply drop it, Convert.ToInt16() will throw OverflowException for out of range values.

3) You shouldn't create a new instance of CultureInfo every time you need it. It may be expansive. Moreover you're using en-US culture, you already have the CultureInfo.InvariantCulture.

3) To know if a value is valid you can use Enum.IsDefined().

To put all together:

public List<T> GetEnumItemsFromUshort<T>(ushort input)
    where T : struct, IComparable, IConvertible, IFormattable
{
    const int bitsInUInt16 = sizeof(ushort) * 8;

    return Enumerable.Range(0, bitsInUInt16 - 1)
        .Select(x => 1 << x)
        .Where(x => (input & x) == x && Enum.IsDefined(typeof(T), x))
        .Select(x => (T)Convert.ChangeType(x, typeof(T)))
        .ToList();
}

It's a minor change and a micro-optimization but I'd also change return type to IEnumerable<T>. If caller will need a list then it will be able to call ToList() itself and if it does not need it then you will avoid an unnecessary copy. Also first Select() may be dropped and embedded in Where() clause.

Note that 0 is not handled, you may want to include it or not; I'd leave it out because it's always matched unless you have input == 0. Note that to do not have a 0 default value for an enum is a dangerous practice, value types are 0 initialized and an uninitialized enum will have an unknown value.

Inverting point of view you may write:

public List<T> GetEnumItemsFromUshort<T>(ushort input)
    where T : struct, IComparable, IConvertible, IFormattable
{
    return Enum.GetValues(typeof(T))
        .Cast<object>()
        .Where(x => ((ushort)x & input) == (ushort)x)
        .Cast<T>()
        .ToList();
}

This 2nd version will also handle shared bits like GuestDefault, UserDefault and AdminDefault in this example:

[Flags]
enum SampleCases {
    None = 0,
    Read = 1,
    Write = 2,
    Execute = 4,
    GuestDefault = Read,
    UserDefault = Read | Execute,
    AdminDefault = Read | Write | Execute
}

Note the hacky .Cast<object>() to use .Where() over a non-generic enumeration (also paying boxing price). Here I tried to keep code short but I wouldn't implement this with LINQ (generated enumerator with yield return will be more clear, IMO). Also this code returns duplicates and aliases, if it's not what you want then you have to handle them (checking for distinct values and for multiple bits set on the same value).

Very last point: your constraints for generic parameter reduce wrong usage of your function but it doesn't really limit usage to enums: most primitive types implement same interfaces. I'd also add an explicit check:

if (!typeof(T).IsEnum))
    throw new ArgumentException("Template argument must be an enum type.");
\$\endgroup\$
  • \$\begingroup\$ I see you are not using the input anywhere in your code, thus returning a list of bit values, this does not meet the same objective as my code, since I want to retrieve which flags are present in the input. I think I am not understanding the code you wrote. \$\endgroup\$ – Oscar Guillamon Nov 17 '15 at 12:33
  • \$\begingroup\$ @OscarGuillamon you're right, I missed out first Where()! Thanks! \$\endgroup\$ – Adriano Repetti Nov 17 '15 at 12:44
  • \$\begingroup\$ Thanks, I will try it ASAP, but it now meets the same purpose as mine, marked as solution, thanks for the tips. Will keep them in mind from now on \$\endgroup\$ – Oscar Guillamon Nov 17 '15 at 12:59
  • \$\begingroup\$ I don't quite understand the second iteration in which you say inverting point of view you may write what is the point of Cast<object> and then recasting to the generic type? \$\endgroup\$ – Oscar Guillamon Nov 17 '15 at 13:41
  • 1
    \$\begingroup\$ You're right! Use of underlying type was there from version with Convert.ChangeType() but it's wrong now, Enum.IsDefined() needs true enum type not underlying type! I'll fix it, thanks! \$\endgroup\$ – Adriano Repetti Nov 18 '15 at 8:31

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.