4
\$\begingroup\$

There's a lot of place in my code where I need to do something like this

public static void read(DataInputStream stream) throws IOException
{
  try
  {
      Object thing = getById(stream.readInt());
  }
  catch(IllegalArgumentException ex)
  {
      throw new IOException(ex); // do not throw IllegalArgumentException if the corrupted data points to invalid things
  }
}

public static Object getById(int id)
{
    for(Object thing : things)
        if(thing.getId() == id)
            return thing;

  throw new IllegalArgumentException("Thing with id " + id + " does not exist!");
}

I like the fact that read() only throws IOExceptions and wrap any detail of internal failure with IOException as having those failures are caused by having corrupted data from the stream. (And corrupted data should return IOException, could return something else but the idea is the same).

What I really don't like is the tedious process of wrapping all my exceptions everytime and incrementing the number of indents I have just to indicate that IllegalArgumentException becomes IOException. An annotation like this

@WrapExcept(IllegalArgumentException.class, ex -> new IOException(ex))
public static void read(DataInputStream stream) throws IOException
{
  Object thing = getById(stream.readInt());
}

would be perfect but I have no idea how it could be possible. The best I could come up with is something along the lines of

@FunctionalInterface
public interface ThrowingCall<R, E extends Throwable>
{
    R call() throws E;
}

public static <R, W extends Throwable, E extends Throwable, T extends Throwable>
R wrapExcept(ThrowingCall<R, T> call, Class<W> exType, Function<W, E> wrapper) throws E, T
{
    try
    {
        return call.call();
    }
    catch(Throwable t)
    {
        if(exType.isInstance(t))
            throw wrapper.apply(exType.cast(t));
        throw t;
    }
}

public static void read() throws IOException
{
    Object thing = wrapExcept(() -> getById(3), IllegalArgumentException.class, ex -> new IOException(ex));
}

Any idea to make this code look likes less verbose? Obviously what I want to improve is the read() call's readability. Also I'm not sure how ThrowingCall could be replaced by something more standard.

\$\endgroup\$
  • \$\begingroup\$ Code Review requires concrete code from a project, with sufficient context for reviewers to understand how that code is used. Pseudocode, stub code, hypothetical code, obfuscated code, and generic best practices are outside the scope of this site. Please take a look at the help center. \$\endgroup\$ – Mast Apr 29 '18 at 9:51
  • \$\begingroup\$ @Mast Can it get migrated to SO? \$\endgroup\$ – Winter Apr 29 '18 at 12:44
  • \$\begingroup\$ @Mast I consider the first two blocks of code the "motivation" for the actual code under review (namely ` ThrowingCall` and ` wrapExcept` ). As such while they are pseudocode, they do not preclude this question from review, IMO (as we have seen from two IMHO great answers) \$\endgroup\$ – Vogel612 Apr 30 '18 at 8:13
3
\$\begingroup\$

Alas, what you would like to do is not possible. You could indeed play around with interceptors in a CDI container or with an AOP framework, and basically change the exception type at runtime. BUT there's one thing you will never get rid of: the static excption typing in the method declaration.

Thus, even if you have

@WrapException(AnotherCheckedException.class)
public void myMethod() throws SomeCheckedException

and an interceptor

@AroundInvoke
public Object wrapMethod(InvocationContext invocationContext) throws Exception {
    try {
        Object res = invocationContext.proceed();
        return res;
    }
    catch(Exception e) {
        throw new AnotherCheckedException(e);
    }
}

The compiler will still require you to handle SomeCheckedException in the method call, as it cannot know that you will replace that exception at runtime.

The only way I see to change this is: do not use checked exceptions at all. Do the local catch-and-rewap dance in all places where some library insists on checked exceptions, and only handle runtime exceptions in your code. Then you have all liberty you need.

The pros-and-cons of replacing all checked with runtime exceptions are definitely beyond the scope of this question. Let's just say: it is a controverse subject...

\$\endgroup\$
  • \$\begingroup\$ That's a good point. No I'm not OK with replacing all checked exceptions with runtime exceptions. Didn't know about the existence of interceptors, thank you! \$\endgroup\$ – Winter Apr 24 '18 at 12:58
  • \$\begingroup\$ You can avoid interceptors and AOP with the native Java Proxy but the checked exception problem explained by @mtj will remain.. \$\endgroup\$ – gervais.b Apr 25 '18 at 14:17
3
\$\begingroup\$

This looks like something Optionals can handle, so:

public static Optional<Object> getById(int id) {
    for(Object thing : things) {
        if(thing.getId() == id) {
            return Optional.of(thing);
        }
    }

    return Optional.empty();
}

Now in your read code, you can simply do this:

public static void read(DataInputStream stream) throws IOException {
    Object thing = getById(stream.readInt())
                       .orElseThrow(() -> new IOException("Could not find Id"));

    // Do something with thing
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.