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?
-
\$\begingroup\$ Did you read the MSDN article on this? You need to provide a type as a parameter when using generic methods. \$\endgroup\$Quill– Quill2016年01月13日 14:27:47 +00:00Commented Jan 13, 2016 at 14:27
-
3\$\begingroup\$ This question is being discussed on meta. \$\endgroup\$Dan– Dan2016年01月13日 15:16:12 +00:00Commented Jan 13, 2016 at 15:16
2 Answers 2
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();
-
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 callingSelectListFor<BrochureTypes>()
\$\endgroup\$Pete– Pete2016年01月13日 14:54:38 +00:00Commented Jan 13, 2016 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– Gentian Kasa2016年01月13日 15:00:03 +00:00Commented Jan 13, 2016 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– Pete2016年01月13日 15:12:46 +00:00Commented Jan 13, 2016 at 15:12
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();
}
-
\$\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– Pete2016年01月13日 15:37:45 +00:00Commented Jan 13, 2016 at 15:37
Explore related questions
See similar questions with these tags.