Throw/do-not-throw an exception based on a parameter - why is this not a good idea?

Question

I was digging around in MSDN and found this article which had one interesting bit of advice: Do not have public members that can either throw or not throw exceptions based on some option.

For example:

Uri ParseUri(string uriValue, bool throwOnError)

Now of course I can see that in 99% of cases this would be horrible, but is its occasional use justified?

One case I have seen it used is with an "AllowEmpty" parameter when accessing data in the database or a configuration file. For example:

object LoadConfigSetting(string key, bool allowEmpty);

In this case, the alternative would be to return null. But then the calling code would be littered with null references check. (And the method would also preclude the ability to actually allow null as a specifically configurable value, if you were so inclined).

What are your thoughts? Why would this be a big problem?

Answer 1

I think it's definitely a bad idea to have a throw / no throw decision be based off of a boolean. Namely because it requires developers looking at a piece of code to have a functional knowledge of the API to determine what the boolean means. This is bad on it's own but when it changes the underlying error handling it can make it very easy for developers to make mistakes while reading code.

It would be much better and more readable to have 2 APIs in this case.

Uri ParseUriOrThrow(string value);

bool TryParseUri(string value, out Uri uri);

In this case it's 100% clear what these APIs do.

Article on why booleans are bad as parameters: http://blogs.msdn.com/jaredpar/archive/2007/01/23/boolean-parameters.aspx

Answer 2

It's usually best to choose one error handling mechanism and stick with it consistently. Allowing this sort of flip-flop code can't really improve the life of developers.

In the above example, what happens if parsing fails and throwOnError is false? Now the user has to guess if NULL if going to be returned, or god knows...

True there's an ongoing debate between exceptions and return values as the better error handling method, but I'm pretty certain there's a consensus about being consistent and sticking with whatever choice you make. The API can't surprise its users and error handling should be part of the interface, and be as clearly defined as the interface.

Answer 3

It's kind of nasty from a readabilty standpoint. Developers tend to expect every method to throw an exception, and if they want to ignore the exception, they'll catch it themselves. With the 'boolean flag' approach, every single method needs to implement this exception-inhibiting semantic.

However, I think the MSDN article is strictly referring to 'throwOnError' flags. In these cases either the error is ignored inside the method itself (bad, as it's hidden) or some kind of null/error object is returned (bad, because you're not using exceptions to handle the error, which is inconsistent and itself error-prone).

Whereas your example seems fine to me. An exception indicates a failure of the method to perform its duty - there is no return value. However the 'allowEmpty' flag changes the semantics of the method - so what would have been an exception ('Empty value') is now expected and legal. Plus, if you had thrown an exception, you wouldn't easily be able to return the config data. So it seems OK in this case.