3
\$\begingroup\$

I saw a question on Stack Overflow asking for a review of a custom try...catch construct that made use of Optionals, and got the idea to try writing my own version. Mine doesn't use Optionals though. Each supplied catch handler is expected to return a value.

I realized about half way through writing this that this is probably an awful idea that should never be used d in the real world for various reasons. I haven't written Java in a few years though, and I'm curious what can be improved.

Example of use:

Integer i =
        new Try<>(()-> Integer.parseInt("f"))
                  .catching(NumberFormatException.class, (e)-> 2)
                  .catching(IllegalStateException.class, (e) -> 3)
                  .andFinally(()-> System.out.println("Finally!"));

// Finally!

Integer j =
        new Try<>(()-> Integer.parseInt("1"))
                  .catching(NumberFormatException.class, (e) -> 2)
                  .execute();

System.out.println(i); // 2
System.out.println(j); // 1

Basically how it works is the Try object can have catching methods chained on it that register handlers. When an exception is thrown, it looks for a corresponding handler, and calls it, returning the returned value. If no handler is found, it rethrows the exception.

Major problems:

  • The need for .class is unfortunate. I couldn't think of how else to have the caller indicate the class of exception to be caught though.

  • Non-exception classes can be registered, although it will have no effect other than the minimal memory and CPU usage.

  • The need for execute when andFinally isn't used. I can't see how the class would know that all catches have been added though.

  • The need for new is unfortunate too. I figured I could take a page from Scala and create a static method that acts as a constructor, but then I'd need to do something like Try.tryMethod to reference it, which isn't a whole lot better.

  • The check in andFinally seems like it's unnecessary. Unless someone does something bizarre like:

    Try t = new Try<>(() -> null);
    t.andFinally(() -> {});
    t.andFinally(() -> {});
    

    It shouldn't be possible to add multiple finally blocks. I'm not sure to what extent I should stop the user from doing stupid things. I could make the object immutable, but again, I'm not sure if that's necessary.

Since this is basically just exercise code, I'd love to anything at all that could be improved here. This is the first Java I've really written in a few years, so I'm sure there's lots that can be improved.

import java.util.HashMap;
import java.util.Map;
import java.util.function.Function;
import java.util.function.Supplier;

public class Try<T> {
    private Supplier<T> tryExpr;
    private Map<Class, Function<Exception, T>> handlers = new HashMap<>();
    private Runnable finallyExpr;

    public Try(Supplier<T> tryExpr) {
        this.tryExpr = tryExpr;
    }

    public Try<T> catching(Class ex, Function<Exception, T> handler) {
        if(handlers.containsKey(ex)) {
            throw new IllegalStateException("exception " + ex.getSimpleName() + " has already been caught");

        } else {
            handlers.put(ex, handler);

            return this;
        }
    }

    public T execute() {
        try {
            return tryExpr.get();

        } catch (Exception e) {
            if(handlers.containsKey(e.getClass())) {
                Function<Exception, T> handler = handlers.get(e.getClass());

                return handler.apply(e);

            } else {
                throw e;
            }

        } finally {
            if(finallyExpr != null) {
                finallyExpr.run();
            }
        }
    }

    public T andFinally (Runnable newFinallyExpr) {
       if (finallyExpr == null) {
           finallyExpr = newFinallyExpr;

       } else {
           throw new IllegalStateException("Cannot have multiple finally expressions");
       }

       return execute();
    }

}
\$\endgroup\$
5
\$\begingroup\$

Thank you for sharing your code and this idea with us! :)

I have just some personal preferences and ideas to some of you "major problems" witch should be additional to the answers of @Mateusz Stefek and @TorbenPutkonen


Not Functional

public Try<T> catching(Class ex, Function<Exception, T> handler) {
  if(/* ... */) {
    throw new IllegalStateException("exception " + ex.getSimpleName() + " has already been caught");
  } else {
    /* ... */
  }
}

In functional programming attempt not to throw any error instead you would return an Either.

new Try<>(()-> Integer.parseInt("1"))
                      .catching(NumberFormatException.class, (e) -> 2)
                      .execute();

On each method call on Try you modify it, but fp is all about immutability.

Some Refactoring

Naming

public Try<T> catching(Class ex, Function<Exception, T> handler)

I would write out ex to exception. Than we can change the name of

private final Map<Class, Function<Exception, T>> handlers

to exceptionByHandler, which makes it easier to read, because it suggests a Map more intuitively, than handler which could be a List.

Null check

We have currently two null checks

// in execute
if(finallyExpr != null)

// in andFinally
if (finallyExpr == null)

The utility class Objects offers the static methods isNull and nonNull and with an static import it could look like

// in execute
if(nonNull(finallyExpr))

// in andFinally
if (isNull(finallyExpr))

But if we use an Optional in andFinally it could look like

public T andFinally(Runnable newFinallyExpr) {
    finallyExpr = Optional.ofNullable(finallyExpr)
                          .orElseThrow(() -> new IllegalStateException("Cannot have multiple finally expressions"));

    return execute();
}

Non Intuitiv API

The check in andFinally seems like it's unnecessary. [...]

You already mention this as a flaw.. but from an other perspective. For me as a client it is not intuitive to have the method andFinally and execute to fire up the Try.

More natural would be to let the client call the execute itself and override all finally with the next occurring finally.

new Try<>(()-> Integer.parseInt("f"))
                  .catching(NumberFormatException.class, (e)-> 2)
                  .catching(IllegalStateException.class, (e) -> 3)
                  .andFinally(()-> System.out.println("Finally!"))
                  .andFinally(()-> System.out.println("Finally #2!")) // overrides all previous finally
                  .execute();

Improvement

Make Try immutable

As in functional programming objects are immutable we can do it in Java too. We can let the client modify a Try with methods we provide him and return each time a new Try.

But to build up a Try we can create a TryBuilder witch is mutable. This is a common way which you can find in the Java-World to or example with String and StringBuilder.

State Pattern

The need for execute when andFinally isn't used. I can't see how the class would know that all catches have been added though.

This is a scenario for the State Pattern. We have our base-state, which allows the client to add multiple catch-cases. After the last catch-case we need to let the client allow only one finally-case. After this our object is in the execution-state.

Builder Pattern

We can mix the State-Pattern with the Builder-Pattern and on each state we can use a builder, which only let use the methods the client currently needs to make the api more intuitive.

Replace the Constructor

The need for new is unfortunate too. I figured I could take a page from Scala and create a static method that acts as a constructor, but then I'd need to do something like Try.tryMethod to reference it, which isn't a whole lot better.

With a static method as constructor we are allowed to return a different reference type.

new Try(/* ... */) // can only return an instance of Try

Try.of(/* ... */)  // can return everything you can imagine 

For the use case I have in mind we need to make the constructor private and return a builder that we need to implement.

private Try(/* ... */) { /* ... */ }

public  static <T> CatchBuilder of(Supplier<T> tryExpr) {
    return new CatchBuilder<T>(new TryBuilder<T>().withTryExpr(tryExpr));
}

Example Implementation

Try

The Try is a public class with builds the API to the client. It provides the client outside of the package only a of(Supplier<T> tryExpr) to build up a Try. The constructor is package private therewith TryBuilder can access it. With of(Supplier<T> tryExpr) the clients gets a CatchBuilder (this is the modified State-Pattern mentioned above)

public class Try<T> {

  private Supplier<T> tryExpr;

  private Map<Class, Function<Exception, T>> handlers;

  private Runnable finallyExpr;

  Try(Supplier<T> tryExpr,
      Map<Class, Function<Exception, T>> handlers,
      Runnable finallyExpr) {
    this.tryExpr = tryExpr;
    this.handlers = handlers;
    this.finallyExpr = finallyExpr;
  }

  public static <T> CatchBuilder of(Supplier<T> tryExpr) {
    return new CatchBuilder<T>(new TryBuilder<T>().withTryExpr(tryExpr));
  }

  public T execute() {
    try {
      return tryExpr.get();
    } catch (Exception e) {
      if (handlers.containsKey(e.getClass())) {
        Function<Exception, T> handler = handlers.get(e.getClass());
        return handler.apply(e);
      } else {
        throw e;
      }
    } finally {
      if (finallyExpr != null) {
        finallyExpr.run();
      }
    }
  }
}

TryBuilder

This is the mutable way to build up a Try.

class TryBuilder<T> {

  private Supplier<T> tryExpr;

  private Map<Class, Function<Exception, T>> handlers;

  private Runnable finallyExpr;

  public TryBuilder<T> withTryExpr(Supplier<T> tryExpr) {
    this.tryExpr = tryExpr;
    return this;
  }

  public TryBuilder<T> withHandlers(Map<Class, Function<Exception, T>> handlers) {
    this.handlers = handlers;
    return this;
  }

  public TryBuilder<T> withFinallyExpr(Runnable finallyExpr) {
    this.finallyExpr = finallyExpr;
    return this;
  }

  public Try<T> build() {
    return new Try<>(tryExpr, handlers, finallyExpr);
  }

}

CatchBuilder and FinallyBuilder

class CatchBuilder<T> {

  private final Map<Class, Function<Exception, T>> handlers = new HashMap<>();

  private final TryBuilder<T> tryBuilder;

  CatchBuilder(TryBuilder<T> tryBuilder) {
    this.tryBuilder = tryBuilder;
  }

  public CatchBuilder<T> withCatch(Class ex, Function<Exception, T> handler) {
    if (handlers.containsKey(ex)) {
      throw new IllegalStateException("exception " + ex.getSimpleName() + " has already been caught");
    }

    handlers.put(ex, handler);
    return this;
  }

  public FinallyBuilder<T> and(Class ex, Function<Exception, T> handler) {
    withCatch(ex, handler);
    return new FinallyBuilder<>(tryBuilder.withHandlers(handlers));
  }

  public FinallyBuilder<T> onlyCatch(Class ex, Function<Exception, T> handler) {
    return and(ex, handler);
  }

}

class FinallyBuilder<T> {
  private final TryBuilder<T> tryBuilder;

  public FinallyBuilder(TryBuilder<T> tryBuilder) {
    this.tryBuilder = tryBuilder;
  }

  public Try<T> finallyDo(Runnable newFinallyExpr) {
    return tryBuilder.withFinallyExpr(newFinallyExpr)
                     .build();
  }

  public Try<T> withoutFinally() {
    return tryBuilder.build();
  }

}

Exmaple

public static void main(String[] args) {

  Try<Integer> firstTry = Try.of(() -> Integer.parseInt("f"))
                             .withCatch(NumberFormatException.class, (e) -> 2)
                             .and(IllegalStateException.class, (e) -> 3)
                             .finallyDo(() -> System.out.println("Finally!"));

  Try<Integer> secondTry = Try.of(() -> Integer.parseInt("f"))
                              .onlyCatch(NumberFormatException.class, (e) -> 2)
                              .withoutFinally();
}
\$\endgroup\$
2
\$\begingroup\$

Your code is fluent, and not really functional.

It solves a problem that Java’s try block doesn’t return a value, but I don’t see a benefit of it over extracting the try-catch block into a separate method/lambda.

To make it more functional, you need to allow operating on the Try object on a higher level. For example, transforming the result before the code is executed. The first step would be extracting the interface. Is it Supplier<T>?

Try writing something similar to Try from Vavr.

(Rant mode) This obsession with fluentism and calling it „functional” is in my opinion a sign of a novice Java 8 programmer, leading to monstrocities of flatMapped, orElseGetted Optional constructs. (/Rant mode)

\$\endgroup\$
1
\$\begingroup\$

Fact: To copy regular try-catch functionality, your implementation needs to check if a handler has been registered for exception's superclass and prevent initialization where superclass hides subclass try-block.

Fact: If you support multiple finally blocks, you should support multiple catch blocks for an exception type. Just for symmetry. This would conflict with previous point, so a design choice has to be made.

Opinion: The mental load from having to remember how yet another try-catch abstraction library works counteracts all benefit we might get from reduced "boilerplate".

Disclaimer: Facts expressed above may or may not be based on actual facts.

Mental exercises like this are an important part of learning. But we need to evaluate the pros and the cons when we're done and not be afraid to scrap the bad ideas, like Lombok or Vavr should have been.

\$\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.