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

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 2

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 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.

Answer 4

Another example in line with this could be set of TryParse methods on some of the value types

bool DateTime.TryParse(string text, out DateTime)

Answer 5

In any public API it is really a bad idea to have two ways to check for a faulty condition because then it becomes non-obvious what will happen if the error occurs. Just by looking at the code will not help. You have to understand the semantics of the flag parameter (and nothing prevents it from being an expression).

If checking for null is not an option, and if I need to recover from this specific failure, I prefer to create a specific exception so that I can catch it later and handle it appropriately. In any other case I throw a general exception.

Answer 6

Having a donTThrowException parameter defeats the whole point of exceptions (in any language). If the calling code wants to have:

public static void Main()
{
        FileStream myFile = File.Open("NonExistent.txt", FileMode.Open, FileAccess.Read);
}

they're welcome to (C# doesn't even have checked exceptions). In Java the same thing would be accomplished with:

public static void main(String[] args) throws FileNotFoundException
{
        FileInputStream fs = new FileInputStream("NonExistent.txt");
}

Either way, it's the caller's job to decide how to handle (or not) the exception, not the callee's.

Answer 7

In the linked to article there is a note that Exceptions should not be used for flow of control - which seems to be implied in the example questsions. Exceptions should reflect Method level failure. To have a signature that it is OK to throw an Error seems like the design is not thought out.

Jeffrey Richters book CLR via C# points out - "you should throw an exception when the method cannot complete its task as indicated by its name".

His book also pointed out a very common error. People tend to write code to catch everything (his words "A ubiquitous mistake of developers who have not been properly trained on the proper use of exceptions tend to use catch blocks too often and improperly. When you catch an exception, you're stating that you expected this exception, you understand why it occurred, and you know how to deal with it.")

That has made me try to code for exceptions that I can expect and can handle in my logic otherwise it should be an error.

Validate your arguments and prevent the exceptions, and only catch what you can handle.