Is it bad practice to return Exceptions from your methods

Question

A lot of the code base methods my peers have written perform their own error handling, usually by catching, notifying, and logging.

In these cases the methods return a boolean, indicating success or failure.

Sometimes though, if a method fails, I want the calling code to know why, and returning a boolean is not enough.

One way around this is to keep the error handling in the methods, but make the method void, and have the methods throw their Exceptions.

However, recently I've got into the habit of returning Exceptions in appropriate methods, usually action methods that could otherwise be void, but where I'd like to know the status of the action.

The advantages of returning an Exception over having a void method throw an Exception is that I think it makes it easier for other programmers to use your library, and, more importantly, it means you can safely call that method without having to worry about catching the Exception

For example, if the method is just void, it might not be instantly obvious that the programmer should handle success or failure.

But if the methods specifically specifies a return type of Exception, then you know you can check success or failure if you want to. But it also means you don't need to worry about catching the error if you don't want to.

Does that make sense? Maybe I haven't used the best example, but generally, is it OK to return Exceptions, or is there a better pattern out there?

cheers

UPDATE

wow, the overwhelming result is no way. I thought so. I must say, doing it (returning an Exception) kinda solved a problem, but it did feel wrong.

So, from some of your answers, the best solution to these specific scenarios (a complex class, or a class with one or more external dependencies (i.e. web service)) seems to be a custom results Class?

UPDATE:

hey guys, really appreciating all the opinions, am reading through everything, and I'm thinking carefully about all the input.

Currently I'm favoring having a void method, throwing the Exceptions, and then catching them on the outside....is that better?

cheers

Answer 1

No, it's completely bastardizing the language (technology, since you just specify .NET).

I would also disagree that it makes your APIs easier for others to use, anybody who has learned and has a basic understanding of programming using a model that includes exceptions will find it harder to use your code.

What happens if, in the future, your method needs to return a value? What if there are more than one error scenarios? If you really want to know more about how the method went then you need to create a class that encapsulates both the result and any additional status information you want.

Answer 2

If you mean something like ...

public Exception MyMethod( string foo )
{
   if( String.IsNullOrEmpty() )
   {
      return new ArgumentNullException( "foo" );
   }
}

... rather than ...

public void MyMethod( string foo )
{
   if( String.IsNullOrEmpty() )
   {
      throw new ArgumentNullException( "foo" )
   }
}

Then absolutely not, it is not okay to do that. You would be completely re-inventing the purpose of an exception and using it as an hresult. Here are some standard best practices.

Another good reason not to is that standard delegates would no longer match your method signature. So, for example, you could not use Action<string> on MyMethod anymore and would need to use Func<string,Exception> instead.

@Andy, per comment, answer too long for a comment: Not really. Don't be so concerned about the caller. He should be being defensive in his design anyway. Think about the semantics of an Exception ... "The execution of this application will stop right here unless somebody knows what to do about this problem." If you can resolve the issue, you should resolve it. If you can't, you have to log it and throw it to the next level, because they may know exactly what to do.

You should handle what you can handle and throw what you can't. By definition, the guy up the call stack has a broader view of the world than you. Your application needs to be resilient in it's ability to deal with exceptions and keep on going. They only way to do that is defensive coding and push issues up to higher levels of information to see if anything can be done.

At the end of the day if the answer is "no", then log the problem to it can be understood, be a good citizen of the box and terminate the application gracefully and live to fight another day. Don't be selfish and try and hide errors for your caller. Be defensive and he will do the same. :)

Check out the Enterprise Library Exception handling block. It think they really articulate a great vision for how to deal with exceptions throughout your architecture.

Answer 3

Ive never seen a method which returns a exception. Generally the method returns a value and that method may throw an exception. But ive never seen code with return new System.InvalidOperationException("Logfile cannot be read-only"); for example...

So to answer your question I would say yes it is bad...

Answer 4

You should never return exceptions. Instead, if your function is sufficiently complex, create a class that is the result of your function. For example, GetPatientList method returns GetPatientListResult. It could have a member of Success. Exceptions are for really uncommon stuff, like a file disappearing mid operation, invalid or null parameters being set for a function, the database going down. Also, it breaks the whole abstraction model- Dog.GoForAWalk should return DogWalkReport, not PossiblyNullDogWalkException.

Answer 5

No.

Exceptions should be something that exist only in exceptional cases (hence the name). The only time I could ever see having a method return an exception would be some type of factory method who's purpose was specifically to create the exception - but even then, it's stretching things.

If you need to return more information than a boolean, create a custom class (not derived from exception) that includes your boolean + other info.

Answer 6

Most definitely an unorthodox use of exceptions. The feature that makes exceptions powerful is that an exception that is thrown can bubble up any number of layers in the call stack. If you simply return it, you lose that property (but also the negative performance impact associated with that).

I really don't see how this is different from simply returning an instance of a class that contains informations about possible errors. The only difference is that returning objects that inherit from the Exception class can cause some confusion in other developers who will read your code later.

So while there's nothing wrong with returning objects holding informations about errors, I would suggest using classes that don't inherit from Exception for that purpose, in order to avoid confusion.

Answer 7

No, I would not consider this a good practice.

Exceptions are well-named: they should be exceptional circumstances, not the norm. It's impossible to write a function this way, because you can't return the fruits of the computation if you're returning an exception.

You shouldn't be handling exceptions in your methods unless you truly have a recovery strategy that's worth implementing. I don't count logging and re-throwing as a recovery strategy.

You can always spare your clients from having to write try/catch blocks by throwing unchecked exceptions.

Answer 8

Exceptions can be nice in some settings, but keep in mind that they can incur a big, big performance hit.

Answer 9

If you are worried about other programmers not catching them, you can use code analysis tools to make sure all the exceptions are caught. It may be even possible to write custom attributes to achieve something like checked exceptions in Java.

Having said that, isn't it easier to follow well-established conventions and just throw an exception? If you put it in a function comment, it should be enough. If you think about some specific programmers not catching exceptions, you've dealing with PKBAC.

Answer 10

Returning an exception in the scenario you described is a bad idea as exceptions are not designed for that purpose.

It would be better to create your own struct or class that contains the info you need and return that instead.

public enum MyErrorType
{
 Abc,
 Xyz,
 Efg
};

public struct Result
{
 public bool Success;
 public MyErrorType ErrorType;
};

and then the method:

private Result MyMethod()
{
   // code
}

Answer 11

I have to agree that, in general, it's a really bad idea.

However, I can think of a scenario or two where it might make sense.

For example, if you are developing a class library project you might want to throw your own custom exceptions, and you might want to do some extra work whenever one of these exceptions is thrown (like phone home if the user allows it).

To enforce this you might want to use a factory pattern, such that only you can create them and you force the creation to go through one place (where your "phone home" code also lives).

In this case, you wouldn't want the factory to throw the exception itself because that would put your stack trace off. Instead, it would return it to the original method to throw.

But situations like this are the **cough** exception, rather than the rule. Under no circumstances would I want to see an exception passed across class library boundries, and under no circumstances would I create an exception without being sure it's going to get thrown.

Answer 12

I'm not sure the harm in returning an exception, as I do it so that I can report the exception message, type, and related information into an exception log on a sql server.

Some 'exceptions' like sql server connection failed for reporting other exceptions I don't need to bubble up to another level; I can just try to write them out to a network drive instead of crashing.

Other types of exceptions I'd just as soon check to see if the method that called the function can handle that specific issue without the performance hit of actually throwing it

In addition for these methods I include a boolean parameter that allows the caller to decide if it should throw the exception or just return it.

Answer 13

No-one else has said this: My understanding is that in .NET, an Exception's stack trace isn't filled in until the Exception is thrown. See Best Practices for Handling Exceptions where it says:

• The stack trace begins at the statement where the exception is thrown and ends at the catch statement that catches the exception. Be aware of this fact when deciding where to place a throw statement.

• Use exception builder methods. It is common for a class to throw the same exception from different places in its implementation. To avoid excessive code, use helper methods that create the exception and return it.

In .NET, if you simply instantiate an Exception and return it, you won't have any stack information in that Exception. This is a benefit when you use this feature to make an Exception builder method, a loss when you use Exceptions the way you are asking about. By comparison, in Java, the stack information is added when the Exception is instantiated, whether or not it is ever thrown.

Thus, in .NET when you return an Exception that was never thrown, you forgo the very stack trace that tells you where the problem occurred. Maybe you don't care, in this instance, but I thought it was worth pointing out.

The performance overhead of a thrown Exception is reasonably small in any modern JVM and in any recent .NET release. Returning an Exception rather than throwing it -- for performance reasons -- is an inappropriate optimization ... unless you have profiled your program and have found that a specific throw truly is a bottleneck, and that returning the Exception makes a meaningful difference. (Which would surprise me, but anything is possible.)

Note: Let's say you make a method that would otherwise be void that returns an Exception on error. This means if you want to check for error, you have to check against null to see whether there was an error or not. As a general rule, methods that return null on error contribute to fragile code. For example, a method that returns a List should return an empty List, rather than null.

Also note that in .NET when you rethrow an Exception, because the stack is filled in at a throw clause, you will usually lose the original stack trace. To avoid this, use the bare throw without specifying the Exception instance. If you throw ex then you will lose the original stack trace.

Answer 14

Exceptions are for just that exceptional situations. Leave it to the one who caused the exceptional case (the caller) to solve the problem. Say if you have a method that divides two number like

int Div(int x, int y) { return x/y; }

how would you implement prober errorhandling for that method? What value would you return to the call in the below cases?

Div(10,0); or(int.MinValue,-1); //result of int.MinValue == int.MaxValue + 1;

There's no 'right' action in the two cases it depends on the usage of the method. Generally it's a good idea to break code execution when an unexpected situation arises.

Error handling is more than try{] catch{}. What ever goes into the catch block needs to make sensible decision based on the error. simply catching the exception and returning it is not errorhandling it's symptom hidding and will make debuging at best very difficult and often nightmarish.