4

I've been coding in java for a few years now, and I really like the idea of not using null when possible, which for the most part is not too difficult, but I keep coming back to the same instance where I can't decide if its better to use a ObjectNotFoundException or just return null. Any body have any thoughts on the best practice here?

Say you have some email subscriptions stored in a database, which you access via a unique key (code):

public EmailSubscriptionModel getSubscriptionByCode(final String code){
   //go get a list of email subscriptions from DB matching code

   if({list is not empty}){
        return list.get(0);
   }else{
        return null;
   }
}

public void subscribe(final String code){
   EmailSubscriptionModel subscription = getSubscriptionByCode(code);

   if(subscription != null){
      //do subscription
   }

 }

The only way I can think of to to avoid this null check is to throw an ObjectNotFoundException in the getSubscriptionByCode() method in which case we'd have to use a try/catch to catch the error like so:

public EmailSubscriptionModel getSubscriptionByCode(final String code) 
   throws ObjectNotFoundException{

      //go get a list of email subscriptions from DB matching code

       if({list is empty}){
          throw new ObjectNotFoundException();
       }

       return list.get(0);
}

public void subscribe(final String code){

   try{
       EmailSubscriptionModel subscription = getSubscriptionByCode(code);
       //do subscription

   }catch(ObjectNotFoundException e){
      //just move on
   }

 }

Is there a reason this would be preferable to the former? or is it just a style choice?

Ryan
  • 414
  • 1
  • 6
  • 16
  • exceptions should only be thrown in exceptional cases - as in, your program is in an unexpected state. If your database is guaranteed (by requirements, by API contract, whatever) to contain records, but you don't see those records, that's cause for an exception. Otherwise, return `null` or a status code of some sort. – Dan O Sep 16 '15 at 18:38
  • possible duplicate of [Should a retrieval method return 'null' or throw an exception when it can't produce the return value?](http://stackoverflow.com/questions/175532/should-a-retrieval-method-return-null-or-throw-an-exception-when-it-cant-prod) – ventsyv Sep 16 '15 at 18:54

4 Answers4

3

Throwing an exception in an expected (= quite likely) situation is bad style. Throwing a checked one is especially unpleasant for the API user.

If you really wan't to avoid null and insist that the client must consider the null case use Optional<EmailSubscriptionModel> (Java 8+).

atamanroman
  • 11,607
  • 7
  • 57
  • 81
  • 1
    Yeah, and in case he’s not using Java 8, returning the `List`, which seems to already exist, is an option. Then the caller may check whether the list is empty. – Holger Sep 16 '15 at 18:41
3

First of all, exceptions are for the exceptional - you should not throw an exception if something like a subscriber isn't found. It's no big deal. Consider a use case of someone wanting to check if they subscribed - if the answer is no (ie their subscription was not found) its not an exception.

Use exceptions to tell the caller that a situation was encountered that can not be handled and the problem of handling it is being delegated to the caller.

Returning null is fine for java 7.

Use Optional for java 8+ for a cleaner API.

Bohemian
  • 412,405
  • 93
  • 575
  • 722
1

The problem with returning Null is that it's not a valid object and forces the caller to check for null. The whole idea behind exceptions is to allow you to write code that implements the "main logic", without being obstructed by a ton of error checking.

I tend to either return newly constructed object or throw an exception. Obviously this approach assumes that you do expect that you should be returning an item and not finding one is an error.

A lot of libraries seem to be following this approach. Obviously you want to provide a way for the user to check if the item exists without throwing an exception if it does not, but if the assumption is that you should be getting something back but you are not, throwing an exception is the better solution.

Occasionally creating a new item and returning it works too. I've seen it when implementing [] operator for hashmaps / dictionaries.

So let's say I'm implementing some sort of container. I would have

boolean isItemAvailable(T someParam);
T getItem(S someParam);

The documented expectation would be that the caller will use it as:

if (isItemAvailable(myParam))
{   
   getItem(someParam);
}
ventsyv
  • 3,316
  • 3
  • 27
  • 49
0

Personally, I think that throwing exceptions in cases where there is actually no error is a bad pattern. It just causes unnecessary overhead and if you expect other exceptions it's unclear which ones are real errors and which ones are normal cases.

Additionally, null is there for a reason. Avoiding it at all cost and throwing exceptions just to catch them just afterwards and proceed with normal processing is not good.

If it's really an error scenario that the list is empty, then throwing an exception is fine though.

Henri Benoit
  • 705
  • 3
  • 10