14
\$\begingroup\$

I have the following enum extension method:

public static IList<SelectListItem> SelectListFor<T>()
{
  Type enumType = typeof(T);

  if (enumType.IsEnum)
  {
    return Enum.GetValues(enumType)
    .Cast<int>()
    .Where(i => !i.Equals(0))
    .Select(e => new SelectListItem()
            {
            Value = e.ToString(),
      Text = GetDisplayName(enumType, Enum.GetName(enumType, e))
    })
    .ToList();
  }

  return null;
}

I can call this by doing

EnumExtensions.SelectListFor<BrochureTypes>()

And it will allow me to create a drop down list for the values in my BrochureTypes enum

However, when I compile the code, I get the following warning:

CA1004 Consider a design where 'EnumExtensions.SelectListFor()' doesn't require explicit type parameter 'T' in any call to it.

Can I somehow change the above code to overcome this warning? Or should I just suppress the warning?

\$\endgroup\$
7
\$\begingroup\$

As written in the relative MSDN page, the cause of the warning is

The parameter signature of an externally visible generic method does not contain types that correspond to all the type parameters of the method.

Looking at your code, you get the type of the generic param into the method body (which can be seen also as a violation of the Separation of Concerns principle). In order to solve the issue, you can pass the type as a parameter. The resulting code should be something like the following:

public static IList<SelectListItem> SelectListFor(Type enumType)
{
    if (enumType.IsEnum)
    {
        return Enum.GetValues(enumType)
            .Cast<int>()
            .Where(i => !i.Equals(0))
            .Select(e => new SelectListItem()
            {
                Value = e.ToString(),
                Text = GetDisplayName(enumType, Enum.GetName(enumType, e))
            })
            .ToList();
    }

    return null;
}

and can be called in the following way:

EnumExtensions.SelectListFor(typeof(BrochureTypes))

As an alternative, you can make it an extension method of the Type class, and I'd suggest a rename of the method in this case:

public static IList<SelectListItem> GetSelectList(this Type enumType)
{
    if (enumType.IsEnum)
    {
        return Enum.GetValues(enumType)
            .Cast<int>()
            .Where(i => !i.Equals(0))
            .Select(e => new SelectListItem()
            {
                Value = e.ToString(),
                Text = GetDisplayName(enumType, Enum.GetName(enumType, e))
            })
            .ToList();
    }

    return null;
}

and call it like this:

(typeof(BrochureTypes)).GetSelectList();

or:

enumInstance.GetType().GetSelectList();
\$\endgroup\$
  • 1
    \$\begingroup\$ This gets around the warning but it just doesn't seem as nice having to pass typeof(BrochureTypes) into a function rather than just calling SelectListFor<BrochureTypes>() \$\endgroup\$ – Pete Jan 13 '16 at 14:54
  • 2
    \$\begingroup\$ @Pete I can agree with you, but from a design point-of-view you are working with a Type object. If a method is working with an object it makes perfect sense to pass it as a parameter. \$\endgroup\$ – Gentian Kasa Jan 13 '16 at 15:00
  • \$\begingroup\$ Ah, nice edit I think I will go with the extension method, but put it onto the actual enum and make it use all but the selected one so that the 0 in !i.Equals(0) doesn't have to be a hard coded value \$\endgroup\$ – Pete Jan 13 '16 at 15:12
7
\$\begingroup\$

Naming

Type enumType = typeof(T);  

this name is misleading because one would assume that its always an enum which you later check by reading the IsEnum property. So just name it type or pay the price and call typeof(T) twice.

General

I usualy tend to return a empty List<T> instead of null because that makes any null check redundant.

I would suggest reverting the if condition to return early. In its current form its not quite obvious (maybe because of the indentation) how/where the method returns.

Warning

I somehow couldn't reproduce the warning here on my side, but as a hacky workaround you can pass a optional parameter to that method.

Implementing the mentioned points will lead to

public static IList<SelectListItem> SelectListFor<T>(T dummy = default(T))
{
    // using the dummy parameter to omit the CA1801 warning
    if (!dummy?.GetType().IsEnum) // C# 6.0 feature
    {
        return new List<SelectListItem>();
    }

    Type enumType = typeof(T);

    return Enum.GetValues(enumType)
        .Cast<int>()
        .Where(i => !i.Equals(0))
        .Select(e => new SelectListItem()
            {
                Value = e.ToString(),
                Text = GetDisplayName(enumType, Enum.GetName(enumType, e))
            })
        .ToList();

}
\$\endgroup\$
  • \$\begingroup\$ Nice, but unfortunately I wouldn't be able to use this as the above would fail on CA1026, meaning I would have to provide an overload with no parameters calling the the above, which would bring us back to the original problem \$\endgroup\$ – Pete Jan 13 '16 at 15:37

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.