Should I use ArgumentException or InvalidEnumArgumentException for an Enum extension method?

Question

I have seen suggestions for both but would like to confirm that this is the best way to do this:

public enum MO
{
   Learn = 0,
   Practice = 1,
   Quiz = 2
}

public static partial class Extensions
{
   public static MO ToMode(this string mode)
   {
      switch (mode)
      {
         case "Learn": return MO.Learn;
         case "Practice": return MO.Practice;
         case "Quiz": return MO.Quiz;
         default: throw new InvalidEnumArgumentException("Unhandled value: " + mode.ToString());
      }
    }
}

I think it can be `NotSupportedException`... maybe answer also here https://stackoverflow.com/questions/12669805/when-to-use-invalidoperationexception-or-notsupportedexception — demo, Apr 28 '19 at 08:14

score 7 · Accepted Answer · answered Apr 28 '19 at 08:34

I would use ArgumentException in this case. The documentation for InvalidEnumArgumentException states:

This exception is thrown if you pass an invalid enumeration value to a method or when setting a property.

which is not what you're doing here. You're passing a string, not an enumeration. The description for ArgumentException looks more appropriate:

The exception that is thrown when one of the arguments provided to a method is not valid.

Enigmativity · Answer 2 · 2019-04-28T08:57:50.907

I'd be inclined to let the framework decide. Here's the way the framework does it:

public static partial class Extensions
{
    public static MO ToMode(this string mode) => (MO)Enum.Parse(typeof(MO), mode);
}

Now that throws an ArgumentException if you pass an invalid string.

Alternatively, you could implement TryParseMode to avoid exceptions entirely:

public static partial class Extensions
{
    private static Dictionary<string, MO> _lookup =
        Enum
            .GetNames(typeof(MO))
            .ToDictionary(n => n, n => n.ToMode());

    public static bool TryParseMode(this string mode, out MO value)
    {
        var found = _lookup.ContainsKey(mode);
        value = found ? _lookup[mode] : default(MO);
        return found;
    }

    public static MO ToMode(this string mode) => (MO)Enum.Parse(typeof(MO), mode);
}

You'd use that like this:

Console.WriteLine("Learn".TryParseMode(out MO m1));
Console.WriteLine("LearnX".TryParseMode(out MO m2));

...which returns:

True
False

Zazaeil · Answer 3 · 2019-04-28T08:43:07.227

2

You shouldn't throw at all. Stay pure.

   public static (bool, MO) ToMode(this string mode)
   {
      switch (mode)
      {
         case "Learn": return (true, MO.Learn);
         case "Practice": return (true, MO.Practice);
         case "Quiz": return (true, MO.Quiz);
         default: return (false, default);
      }
    }

...let your caller to check the flag and decide what to do.

Other way to do that is to have MO.Unset option serving as default "not recognized" label. Then you simply return your MO.Unset whenever input argument is unexpected.

Keep your code pure, young jedi.

P.S. It is always good practice to have a default value on the enum even if you won't use it in a way I've recommended: enum's are value type and thus always have non-null default value; in case you won't provide one, it is quite possible to mess up.

edited Apr 28 '19 at 08:43

answered Apr 28 '19 at 08:37

Zazaeil

3,900
2
14
31

I'm not sure why the down vote. Even Microsoft [Enum Design](https://learn.microsoft.com/en-us/dotnet/standard/design-guidelines/enum) documentation states *DO provide a value of zero on simple enums. Consider calling the value something like "None." If such a value is not appropriate for this particular enum, the most common default value for the enum should be assigned the underlying value of zero.* which is what it appears you are trying to convey. – Erik Philips Apr 28 '19 at 09:04
Another possibility is to make the return type `MO?` and return `null` if an error occurs. This also has the advantage that you could use the null-coalescing operator easily control what should happen if null is returned. – ckuri Apr 28 '19 at 12:57
Throwing exceptions is not a bad thing by itself. If done right, such as in this case, it's a legitimate design. – Tsahi Asher Apr 28 '19 at 13:34

Should I use ArgumentException or InvalidEnumArgumentException for an Enum extension method?

3 Answers3