115

I'm currently writing some code for UnconstrainedMelody which has generic methods to do with enums.

Now, I have a static class with a bunch of methods which are only meant to be used with "flags" enums. I can't add this as a constraint... so it's possible that they'll be called with other enum types too. In that case I'd like to throw an exception, but I'm not sure which one to throw.

Just to make this concrete, if I have something like this:

// Returns a value with all bits set by any values
public static T GetBitMask<T>() where T : struct, IEnumConstraint
{
    if (!IsFlags<T>()) // This method doesn't throw
    {
        throw new ???
    }
    // Normal work here
}

What's the best exception to throw? ArgumentException sounds logical, but it's a type argument rather than a normal argument, which could easily confuse things. Should I introduce my own TypeArgumentException class? Use InvalidOperationException? NotSupportedException? Anything else?

I'd rather not create my own exception for this unless it's clearly the right thing to do.

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • I stumbled across this today in writing a generic method where extra requirements are placed on the type being used that cannot be described with constraints. I was surprised not to find an exception type already in the BCL. But this exact dilemma was one I also faced a few days ago in the same project for a generic that'll only work with a Flags attribute. Spooky! – Andras Zoltan Mar 01 '12 at 13:09

11 Answers11

51

NotSupportedException sounds like it plainly fits, but the documentation clearly states that it should be used for a different purpose. From the MSDN class remarks:

There are methods that are not supported in the base class, with the expectation that these methods will be implemented in the derived classes instead. The derived class might implement only a subset of the methods from the base class, and throw NotSupportedException for the unsupported methods.

Of course, there's a way in which NotSupportedException is obviously good enough, especially given its common-sense meaning. Having said that, I'm not sure if it's just right.

Given the purpose of Unconstrained Melody ...

There are various useful things that can be done with generic methods/classes where there's a type constraint of "T : enum" or "T : delegate" - but unfortunately, those are prohibited in C#.

This utility library works around the prohibitions using ildasm/ilasm ...

... it seems like a new Exception might be in order despite the high burden of proof we justly have to meet before creating custom Exceptions. Something like InvalidTypeParameterException might be useful throughout the library (or maybe not - this is surely an edge case, right?).

Will clients need to be able to distinguish this from BCL Exceptions? When might a client accidentally call this using a vanilla enum? How would you answer the questions posed by the accepted answer to What factors should be taken into consideration when writing a custom exception class?

Community
  • 1
  • 1
Jeff Sternal
  • 47,787
  • 8
  • 93
  • 120
  • In fact it's almost tempting to throw an internal-only exception in the first place, in the same way that Code Contracts does... I don't believe anyone *should* be catching it. – Jon Skeet Sep 11 '09 at 19:21
  • Too bad it can't just return null! – Jeff Sternal Sep 11 '09 at 19:22
  • 27
    I'm going with TypeArgumentException. – Jon Skeet Sep 11 '09 at 20:01
  • Adding exceptions to the Framework may have a high "burden of proof", but defining custom exceptions shouldn't. Things like `InvalidOperationException` are icky, because "Foo asks collection Bar to add something that already exists, so Bar throws IOE" and "Foo asks collection Bar to add something, so Bar calls Boz which throws IOE even though Bar isn't expecting it to" will both throw the same exception type; code which is expecting to catch the first won't be expecting the latter. That having been said... – supercat May 06 '13 at 17:48
  • ...I would think the argument in favor of a Framework exception here is more compelling than for a custom exception. The general nature of NSE is that when a reference to an object as a general type, and some but not all of the specific types of object to which the reference points will support an ability, attempting to use the ability on a specific type which doesn't support it should throw NSE. I would consider a `Foo` to be a "general type", and `Foo` to be a "specific type" in that context, even though there's no "inheritance" relation between them. – supercat May 06 '13 at 17:52
27

I would avoid NotSupportedException. This exception is used in the framework where a method is not implemented and there is a property indicating that this type of operation is not supported. It doesn't fit here

I think InvalidOperationException is the most appropriate exception you could throw here.

JaredPar
  • 733,204
  • 149
  • 1,241
  • 1,454
  • Thanks for the heads-up about NSE. Would welcome input from your colleagues too, btw... – Jon Skeet Sep 11 '09 at 18:52
  • The point is, the functionality Jon needs has nothing similar in the BCL. The compiler is supposed to catch it. If you remove the "property" requirement from NotSupportedException, things you mentioned (like ReadOnly collection) are the closest thing to Jon's issue. – Mehrdad Afshari Sep 11 '09 at 18:54
  • One point - I do have an IsFlags method (it has to be a method to be generic) which is *sort* of indicating that this type of operation is not supported... so in that sense NSE would be appropriate. i.e. the caller *can* check first. – Jon Skeet Sep 11 '09 at 18:54
  • @Jon: I think even if you don't have such a property **but** all members of your type inherently rely on the fact that `T` is an `enum` decorated with `Flags`, it would be valid to throw NSE. – Mehrdad Afshari Sep 11 '09 at 18:56
  • In fact I *could* just go ahead and give an answer anyway, in all cases - but I don't like giving meaningless results when the developer should be made aware that they're trying to do bitwise operations on a non-bitwise enum... – Jon Skeet Sep 11 '09 at 19:00
  • @Jon, I can see that argument. It still feels a bit wrong to use it because I worry that a static methods constrained to a type don't have the ... cohesive ... feel that a single interface does. It's the cohesive nature of the interface that allows for the relationship between the property indicating support and the method. – JaredPar Sep 11 '09 at 19:10
  • Yup. InvalidOperationException still doesn't feel right either though. Trouble is, I really don't like creating my own exceptions if I can help it. Hmm. Might just do it anyway - at least for the moment. – Jon Skeet Sep 11 '09 at 19:21
  • 1
    @Jon: `StupidClrException` makes a fun name ;) – Mehrdad Afshari Sep 11 '09 at 19:31
  • @Jon: Here's Brad Abrams's take on `NotImplementedException` and `NotSupportedException`: http://blogs.msdn.com/brada/archive/2004/07/29/201354.aspx – LukeH Sep 11 '09 at 20:17
  • My interpretation of the semantics of `NIE`/`NSE` is that throwing them shouldn't be contingent on the arguments used (whether standard or type arguments). A method is either implemented/supported or it isn't: If it doesn't throw `NIE`/`NSE` in *all* circumstances then it should *never* throw them. – LukeH Sep 11 '09 at 20:38
  • `InvalidOperationException` means that it's invalid for the object's *current state.* But there is no state in which the same argument wouldn't cause the exception. – Scott Hannen Jun 26 '19 at 22:59
13

Generic programming should not throw at runtime for invalid type parameters. It should not compile, you should have a compile time enforcement. I don't know what IsFlag<T>() contains, but perhaps you can turn this into a compile time enforcement, like trying to create a type that is only possible to create with 'flags'. Perhaps a traits class can help.

Update

If you must throw, I'd vote for InvalidOperationException. The reasoning is that generic types have parameters and errors related to (method) parameters are centered around the ArgumentException hierarchy. However, the recommendation on ArgumentException states that

if the failure does not involve the arguments themselves, then InvalidOperationException should be used.

There is at least one leap of faith in there, that method parameters recommendations are also to be applied to generic parameters, but there isn't anything better in the SystemException hierachy imho.

l33t
  • 18,692
  • 16
  • 103
  • 180
Remus Rusanu
  • 288,378
  • 40
  • 442
  • 569
  • 1
    No, there's no way that this can be constrained at compile time. `IsFlag` determines whether the enum has `[FlagsAttribute]` applied to it, and the CLR doesn't have constraints based on attributes. It would in a perfect world - or there'd be some other way to constrain it - but in this case it just doesn't work :( – Jon Skeet Sep 11 '09 at 18:59
  • (+1 for the general principle though - I'd love to be *able* to constrain it.) – Jon Skeet Sep 11 '09 at 19:00
10

I would use NotSupportedException as that is what you are saying. Other enums than the specific ones are not supported. This would of course be stated more clearly in the exception message.

Robban
  • 6,729
  • 2
  • 39
  • 47
  • 2
    NotSupportedException is used for a very different purpose in the BCL. It doesn't fit here. http://blogs.msdn.com/jaredpar/archive/2008/12/12/notimplementedexception-vs-notsupportedexception.aspx – JaredPar Sep 11 '09 at 18:49
10

Apparently, Microsoft uses ArgumentException for that, as demonstrated on example of Expression.Lambda<>, Enum.TryParse<> or Marshal.GetDelegateForFunctionPointer<> in Exceptions section. I couldn't find any example indicating otherwise, either (despite searching local reference source for TDelegate and TEnum).

So, I think it's safe to assume that at least in Microsoft code it's a common practice to use ArgumentException for invalid generic type arguments aside from basic variable ones. Given that the exception description in docs doesn't discriminate between those, it's not too much of a stretch, either.

Hopefully it decides the question things once and for all.

Alice
  • 585
  • 5
  • 16
  • 2
    A single example in the framework isn't enough for me, no - given the number of places where I think MS has made poor choice in other cases :) I wouldn't derive `TypeArgumentException` from `ArgumentException`, simply because a type argument isn't a regular argument. – Jon Skeet Feb 15 '16 at 07:48
  • 1
    That's certainly more compelling in terms of "it's what MS does consistently". It doesn't make it any more compelling in terms of matching the documentation... and I know there are plenty of people in the C# team who care deeply about the difference between regular arguments and type arguments :) But thanks for the examples - they're very helpful. – Jon Skeet Feb 15 '16 at 09:15
  • @Jon Skeet: Made an edit; now it includes 3 examples from different MS libraries, all with ArgumentException documented as the one thrown; so if it's a poor choice, at least it's a consistent poor choice. ;) I guess Microsoft assumes that regular arguments and type arguments are both arguments; and personally, I think such assumption is quite reasonable. ^^' – Alice Feb 15 '16 at 09:23
  • Ah, nevermind, seems you noticed that already. Glad I could help. ^^ – Alice Feb 15 '16 at 09:24
  • I think we'll have to agree to disagree on whether it's reasonable to treat them the same. They're certainly not the same when it comes to reflection, or language rules, etc... they're handled *very* differently. – Jon Skeet Feb 15 '16 at 09:25
  • @Jon Skeet: Fair enough. For me, as a dev getting unhandled ArgumentException, it means "fix that argument!!1" and from my standpoint it makes little difference between replacing invalid regular or generic type argument; in both cases it's a bunch of keystrokes. I can understand the need for distinction, though, especially when the cause of exception is not to be corrected by dev, but handled by program. – Alice Feb 15 '16 at 10:08
8

I'd go with NotSupportedException. While ArgumentException looks fine, it's really expected when an argument passed to a method is unacceptable. A type argument is a defining characteristic for the actual method you want to call, not a real "argument." InvalidOperationException should be thrown when the operation you're performing can be valid in some cases but for the particular situation, it's unacceptable.

NotSupportedException is thrown when an operation is inherently unsupported. For instance, when implementing an interface where a particular member doesn't make sense for a class. This looks like a similar situation.

Mehrdad Afshari
  • 414,610
  • 91
  • 852
  • 789
3

Id go with NotSupportedExpcetion.

Carl Bergquist
  • 3,894
  • 2
  • 25
  • 42
2

Throwing a custom made exception should always be done in any case where it is questionable. A custom exception will always work, regardless of the API users needs. The developer could catch either exception type if he does not care, but if the developer needs special handling he will be SOL.

  • Also the developer should document all the exceptions thrown in the XML comments. –  Sep 11 '09 at 19:38
1

I'm always wary of writing custom exceptions, purely on the grounds that they aren't always documented clearly and cause confusion if not named correctly.

In this case I would throw an ArgumentException for the flags check failure. It's all down to preference really. Some coding standards I've seen go as far as to define which types of exceptions should be thrown in scenarios like this.

If the user was trying to pass in something which wasn't an enum then I would throw an InvalidOperationException.

Edit:

The others raise an interesting point that this is not supported. My only concern with a NotSupportedException is that generally those are the exceptions that get thrown when "dark matter" has been introduced to the system, or to put it another way, "This method must go into the system on this interface, but we won't turn it on until version 2.4"

I've also seen NotSupportedExceptions be thrown as a licensing exception "you're running the free version of this software, this function is not supported".

Edit 2:

Another possible one:

System.ComponentModel.InvalidEnumArgumentException

The exception thrown when using invalid arguments that are enumerators.

Peter
  • 1,776
  • 13
  • 20
  • I'll have it constrained to be an enum (after some jiggery pokery) - it's only the flags I'm worried about. – Jon Skeet Sep 11 '09 at 18:42
  • I think those licensing guys should throw an instance of a `LicensingException` class inheriting from `InvalidOperationException`. – Mehrdad Afshari Sep 11 '09 at 18:50
  • I agree Mehrdad, Exceptions are unfortunately one of those areas where there is a lot of grey in the framework. But I'm sure this is the same for lots of languages. (not saying i'd go back to vb6's runtime error 13 hehe) – Peter Sep 11 '09 at 18:54
1

I'd also vote for InvalidOperationException. I did an (incomplete) flowchart on .NET exception throwing guidelines based on Framework Design Guidelines 2nd Ed. awhile back if anyone's interested.

TrueWill
  • 25,132
  • 10
  • 101
  • 150
0

How about inheriting from NotSupportedException. While I agree with @Mehrdad that it makes the most sense, I hear your point that it doesn't seem to fit perfectly. So inherit from NotSupportedException, and that way people coding against your API can still catch a NotSupportedException.

BFree
  • 102,548
  • 21
  • 159
  • 201