Interesting thought problem on refactoring code that returns a null

307 Views Asked by At

I am interested in hearing your feedback. I've recently seen some Java code that is implemented in the following way:

Object1 SomeMethod(String key) {
    Object1 object1 = null;
    List<Object1> objectList = getAllRecordsWithKeyFromDatabase(key);
    if (!objectList.isEmpty()) {
        object1 = objectList.get(0);
    }
    return object1;
}

void AnotherMethod() {
    ...
    Object1 object1 = SomeMethod(key);
    if (object1 == null) {
        // throw exception
    }
    // continue working
}

I am always concerned whenever a null is returned without context. I would rather the response from SomeMethod be more explicit and was considering refactoring it. Throwing an exception from SomeMethod could offer a way to do this. It would be in context and occur at the point of failure.

I am wondering if there is another way where SomeMethod could inform AnotherMethod that 'nothing was found in the database' instead of assuming that null always equaled 'not found'. I thought that a NullObject could be used, but it is unclear to me how AnotherMethod should avoid 'continue working' if the data was not found.

How would you refactor the code?

3

There are 3 best solutions below

1
On BEST ANSWER

I don't see a problem with it as it, except that I would clearly document that SomeMethod can return null in some cases.

However, if finding null is clearly an exceptional case, then throwing an exception is the right approach. If you change AnotherMethod so that it declares a checked exception, then your intent would be much clearer to users of that method. Something like this:

void AnotherMethod() throws SomethingBadHappenedException {
  //snip
}
0
On

Null object is not the be all end all. It is appropriate for some cases (Collections.emptyXXX() is a good example of this), but sometimes you have to distinguish between something and nothing. If returning nothing is a valid state, then it should return null.

Exceptions are for exceptional cases, i.e. things that should not happen under normal circumstances. Catching and handling exceptions is much more difficult than a check for null.

1
On

I once worked on a project where query results were very naturally collections. But there were special cases where the collection should be empty; other cases where the collection should contain exactly one element.

These queries were being performed on objects that sometimes required faulting from a database.

The approach we finally converged upon was to have our results be returned wrapped in our own type (as opposed to ArrayList or something). To illustrate:

public interface Results<T> implements Iterable<T> {
  Collection<T> all();
  Iterator<T> iterator();
  /**
   * @return one value from the result, or null if the result is empty.
   */
  T ifAny();

  /**
   * @return one value from the result.
   */
  T one() throws EmptyResultException;

  /**
   * @return if the result is empty, returns null, 
   *         if the result has one value, returns the value,
   *         if the result has more than one value, throws.
   */
  T unique() throws MultiValueResultException;

  /**
   * @return the value if the result has exactly one; throws otherwise
   */
  T exact() throws NoExactResultException;
}

You can use a similar approach in your case, if the semantics are important to you:

public final class Result<T> {
  private static final Object NON = new Object();

  private final Object _value;

  public Result(T value) {
    if (value == null) {
      throw ...
    }
    _value = value;
  }

  public T get() {
    return (_value == NON) ? null : (T) _value;
  }

  public T use() {
    if (_value == NON) {
      throw ...
    }
    return (T) _value;
  }
}

Note that Scala idioms use Option to similar effect. Here's one of many links on that side topic: http://www.codecommit.com/blog/scala/the-option-pattern