Calling a method that always throws

Kasper complains that it's hard to call methods that always throw. In particular, code like this won't compile because findGetter is missing a return statement:

  public Method findGetter() throws CsvException {
    try {
      return type.getMethod("get" 
        + fieldName.substring(0, 1).toUpperCase() 
        + fieldName.substring(1));
    } catch(SecurityException e) {
      noSuchGetter(e);
    } catch (NoSuchMethodException e) {
      noSuchGetter(e);
    }
  }

  public void noSuchGetter(Exception cause) throws CsvException {
    String message = String.format("Unable to lookup getter" 
        + " for field %s on %s", fieldName, type.getName());
    throw new CsvException(message, cause);
  }

The problem is that noSuchGetter always throws, but javac doesn't consider that when building this code. Making that method private or final doesn't help -- javac does not look at the body of a method when deciding whether the callers are valid.

Adding return null; to the end of this method appeases the compiler, but it sure isn't pretty:

  public Method findGetter() throws CsvException {
    try {
      return type.getMethod("get" 
        + fieldName.substring(0, 1).toUpperCase() 
        + fieldName.substring(1));
    } catch(SecurityException e) {
      noSuchGetter(e);
    } catch (NoSuchMethodException e) {
      noSuchGetter(e);
    }
    return null; /* never executes! (this method never returns null.) */
  }

As Kasper explains, this workaround sucks! In particular, it suggests to the code reader that he should check for null when calling this method. We need to add a big ugly comment to flag what's going on. Another workaround is to throw AssertionErrors on unreachable lines. This is an improvement but we still have dead code in our program. This line won't ever execute, adding noise to code coverage reports.

Fortunately, there's a clever fix. Declare that noSuchGetter returns a RuntimeException and prefix call to it with throw:

  public Method findGetter() throws CsvException {
    try {
      return type.getMethod("get" 
        + fieldName.substring(0, 1).toUpperCase() 
        + fieldName.substring(1));
    } catch(SecurityException e) {
      throw noSuchGetter(e);
    } catch (NoSuchMethodException e) {
      throw noSuchGetter(e);
    }
  }

  public CsvException noSuchGetter(Exception cause) 
      throws CsvException {
    String message = String.format("Unable to lookup getter" 
        + " for field %s on %s", fieldName, type.getName());
    throw new CsvException(message, cause);
  }

Now the code reader can see that the noSuchGetter calls always throw without reading that method's body. And the compiler will complain if you add any code after that call. We can optionally tweak this so noSuchGetter returns CsvException rather than throwing it.

Methods that always throw should return what they throw. If the method always throws a RuntimeException, we'd return that type.

# posted by Jesse Wilson on Thursday, May 22, 2008