Is returning null after exception is caught bad design [closed]_问答_开发者

Closed. This question is opinion-based. It is not c开发者_StackOverflow社区urrently accepting answers.

Want to improve this question? Update the question so it can be answered with facts and citations by editing this post.

Closed 4 years ago.

Improve this question

I always come across the same problem that when an exception is caught in a function that has a non-void return value I don't know what to return. The following code snippet illustrates my problem.

public Object getObject(){
  try{
    ...
    return object;
  }
  catch(Exception e){
    //I have to return something here but what??
    return null; // is this a bad design??
  }
}

So my questions are:

Is return null bad design?
If so what is seen as a cleaner solution??

thanks.

I would say don't catch the exception if you really can't handle it. And logging isn't considered handling an error. Better to bubble it up to someone who can by throwing the exception.

If you must return a value, and null is the only sensible thing, there's nothing wrong with that. Just document it and make it clear to users what ought to be done. Have a unit test that shows the exception being thrown so developers coming after you can see what the accepted idiom needs to be. It'll also test to make sure that your code throws the exception when it should.

I always come across the same problem that when an exception is caught in a function that has a non-void return value I don't know what to return.

If you don't know what to return, then it means that you don't know how to handle the exception. In that case, re-throw it. Please, don't swallow it silently. And please, don't return null, you don't want to force the caller of the code to write:

Foo foo = bar.getFoo();
if (foo != null) {
    // do something with foo
}

This is IMHO a bad design, I personally hate having to write null-checks (many times, null is used where an exception should be thrown instead).

So, as I said, add a throws clause to the method and either totally remote the try/catch block or keep the try/catch if it makes sense (for example if you need to deal with several exceptions) and rethrow the exception as is or wrap it in a custom exception.

Related questions

How to avoid “!= null” statements in Java?

Above all I prefer not to return null. That's something that the user has to explicitly remember to handle as a special case (unless they're expecting a null - is this documented). If they're lucky they'll deference it immediately and suffer an error. If they're unlucky they'll stick it in a collection and suffer the same problem later on.

I think you have two options:

throw an exception. This way the client has to handle it in some fashion (and for this reason I either document it and/or make it checked). Downsides are that exceptions are slow and shouldn't be used for control flow, so I use this for exceptional circumstances (pun intended)
You could make use of the NullObject pattern.

I follow a coding style in which I rarely return a null. If/when I do, that's explicitly documented so clients can cater for it.

Exceptions denote exceptional cases. Assuming your code was supposed to return an object, something must have gone wrong on the way (network error, out of memory, who knows?) and therefore you should not just hush it by returning null.

However, in many cases, you can see in documentation that a method returns a null when such and such condition occurs. The client of that class can then count on this behaviour and handle a null returned, nothing bad about that. See, in this second usage example, it is not an exceptional case - the class is designed to return null under certain conditions - and therefore it's perfectly fine to do so (but do document this intended behaviour).

Thus, at the end of the day, it really depends on whether you can't return the object because of something exceptional in your way, or you simply have no object to return, and it's absolutely fine.

I like the responses that suggest to throw an exception, but that implies that you have designed exception handling into the architecture of your software.

Error handling typically has 3 parts: detection, reporting, and recovery. In my experience, errors fall into classes of severity (the following is an abbreviated list):

Log for debug only
Pause whatever is going on and report to user, waiting for response to continue.
Give up and terminate the program with an apologetic dialogue box.

Your errors should be classified and handling should be as generically and consistently as possible. If you have to consider how to handle each error each time you write some new code, you do not have an effective error handling strategy for your software. I like to have a reporting function which initiates user interaction should continuation be dependent on a user's choice.

The answer as to whether to return a null (a well-worn pattern if I ever saw one) then is dependent on what function logs the error, what can/must the caller do if the function fails and returns the null, and whether or not the severity of the error dictates additional handling.

Exceptions should always be caught by the controller in the end.
Passing a <null> up to the controller makes no sense.
Better to throw/return the original exception up the stack.

It's your code and it's not bad solution. But if you share your code you Shoudn't use it because it can throw unexpected exception (as nullpointer one).

You can of course use

public Object getObject() throws Exception {}

which can give to parent function usable information and will warn that something bad can happen.

Basically I would ditto on Duffymo, with a slight addition:

As he says, if your caller can't handle the exception and recover, then don't catch the exception. Let a higher level function catch it.

If the caller needs to do something but should then appropriately die itself, just rethrow the exception. Like:

SomeObject doStuff()
  throws PanicAbortException
{
  try
  {
    int x=functionThatMightThrowException();
    ... whatever ...
    return y;
  }
  catch (PanicAbortException panic)
  {
    cleanUpMess();
    throw panic; // <-- rethrow the exception
  }
}

You might also repackage the exception, like ...

catch (PanicAbortException panic)
{
  throw new MoreGenericException("In function xyz: "+panic.getMessage());
}

This is why so much java code is bloated with if (x!=null) {...} clauses. Don't create your own Null Pointer Exceptions.

I would say it is a bad practice. If null is received how do I know if the object is not there or some error happened?

My suggestion is

never ever return NULL if the written type is an array or Collection. Instead, return an empty Collection or an empty array.

When the return type is an object, it is up to you to return null depending on the scenario. But never ever swallow an exception and return NULL.

Also if you are returning NULL in any scenario, ensure that this is documented in the method.

As Josha Blooch says in the book "Effective Java", the null is a keyword of Java.

This word identifies a memory location without pointer to any other memory location: In my opinion it's better to coding with the separation of behavior about the functional domain (example: you wait for an object of kind A but you receive an object of kind B) and behavior of low-level domain (example: the unavailability of memory).

In your example, I would modify code as :

public Object getObject(){
  Object objectReturned=new Object();
  try{
    /**business logic*/
  }
  catch(Exception e){
    //logging and eventual logic leaving the current ojbect (this) in a consistent state
  }
  return objectReturned;
}

The disadvantage is to create a complete Object in every call of getObject() (then in situation where the object returned is not read or write).

But I prefer to have same object useless than a NullPointerException because sometimes this exception is very hard to fix.

Some thoughts on how to handle Exceptions

Whether returning null would be good or bad design depends on the Exception and where this snippet is placed in your system.
If the Exception is a NullPointerException you probably apply the catch block somewhat obtrusive (as flow control).
If it is something like IOException and you can't do anything against the reason, you should throw the Exception to the controller.
If the controller is a facade of a component he, translate the Exception to well documented component-specific set of possible Exceptions, that may occur at the interface. And for detailed information you should include the original Exception as nested Exception.