What exception type to throw for unrecognized ID paramater?

Question

ITool GetTool(Guid tool)
{
    if (tool == Hammer.Id)
        return new Hammer();
    else if (tool == Drill.Id)
        return new Drill();

    else
        throw new ....?
}

What's the most appropriate exception type to throw here? NotSupportedException is the closest I've found but I don't think that's quite right.

Answer 1

Well if you can't find a truly right one, you can always create your own exception:)

Edit: I think ArgumentException is a close one.

Answer 2

I'd go for a simple ArgumentException or ArgumentOutOfRangeException.

The description for the latter sounds just right to me:

The exception that is thrown when the value of an argument is outside the allowable range of values as defined by the invoked method.

If you consider a set of allowable values to be a "range" then that's fine, I'd say.

Do you definitely have to use a Guid here? Could you have an enum of valid tools instead?

EDIT: To answer the suggestion of creating your own exception: what value would that provide? Would you actually want to catch that specific exception? If not, where's the benefit? If you would catch that exception, shouldn't you be validating your arguments before calling the method? I find it's rarely worth creating a custom exception unless there's really nothing which fits.

Answer 3

At first blush, I'd say ArgumentException.

Answer 4

You should use ArgumentException since the problem is the Guid argument "tool" passed doesn't meet any of the supported types for this method. The error message should state as such.

It seems to me, though, that the reason this bugs you and is difficult to tell, is the awkward nature of the case. This method seems to be working sort of like a factory but for specific types that might implement this interface. Shouldn't Hammer implement its own GetTool and always return an ITool of type Hammer? Otherwise you get what you have here which is an attempt to anticipate what all types might come through. That seems destined for trouble should types change or new ones be added. Then the onus is on the original method to support the new type, instead of the new type supporting this common interface method. Isn't that the goal of "pluggable" classes, which is what interfaces sort of encourage?

Answer 5

By process of elimination:

• making your own implies that the exception is catchable and could be handled. It should never be caught, you can't recover from bugs without recompiling.
• NotSupportedException implies that your code is at fault for not supporting the requested tool. Not likely, a Guid has trillions of invalid tool values.
• ArgumentOutOfRange implies that the Guid is too small or too large. Doesn't make sense, they don't have a range.
• ApplicationException is now considered not best-practice and is not specific enough.

ArgumentException("Unknown tool requested")