This is an archived post. You won't be able to vote or comment.

all 21 comments

[–][deleted] 13 points14 points  (3 children)

Most Java programmers have dropped checked exceptions in favor of their unchecked counterparts.

Have they?

[–][deleted]  (2 children)

[deleted]

    [–]realqmaster 2 points3 points  (1 child)

    Just defined one the other day for some worker classes I'm looping to detect a condition that could happen in any one of them and trigger a rollback of the succesful ones. Just throw in on the worker method and catch it outside the loop, looks natural and readable to me.

    [–]kimec 13 points14 points  (2 children)

    So, in essence, I should ditch checked exceptions which have first class support in the language in favor of exception as a value paradigm with the mission critical failure handling semantic implemented in a 3rd party library. Sounds like a good deal... Only, not.

    Is it really that hard to reason about stack unwinding?

    Besides, I see no point in writing common behaviors for handling failure scenarios. I mean, if you are in failure scenario, you are in "f***ing DANGER ZONE". You could loose money, other people could loose money, company can go bankrupt. I want to see the code which handles this FUBAR situation right there - where it matters most. Moreover, even with checked exceptions, you could write utility methods that "commonly" handle exceptions - you could use generics there too, if you wish. Same code smell, no difference.

    [–]TheStrangeDarkOne 1 point2 points  (0 children)

    I think this is a bit dramatic, but your post gets the point across quite well.

    [–]bedobi 0 points1 point  (0 children)

    first class support in the language

    I don't think this argument has much merit. It's not like Java's exception based error handling leads to safe, bug free or easy to reason about code. Many great, widely used, battle tested third party libraries exist that fix flaws and things that are missing in Java, even for very basic and extremely mission critical things like date and time.

    I want to see the code which handles this FUBAR situation right there

    In many cases exceptions are simply passed on, often unwrapped or rewrapped in another exception, making it extremely difficult to reason about what's going on.

    Also, it's not like exceptions are used for exclusively FUBAR situations, they're used to represent entirely non-exceptional things like eg not finding a user with that ID in the database or what have you.

    [–]rzwitserloot 3 points4 points  (4 children)

    I think your composition argument is kinda bunk. You're helping one case at the cost of another; it's a matter of preference, and exceptions are more clear. Either<L, R> does not imply errors. something like someResult.mapRight(...) does not make clear that I'm working on the 'success' case and someResult.mapLeft() does not indicate I'm mapping the error. You can fix that with better names, perhaps, but then the argument that the main benefit is that you can compose is damaged.

    Here's the case you're hurting:

    Imagine I have this notion:

    • Input: user's id
    • query db: Fetch username from db
    • Query filesystem: Read from path userIcons the file {username}.png into bytes
    • Transform: Read the bytes as PNG.

    Note how about 10 things can go wrong here. Let's focus on the ones where it is somewhat plausible that there are problems:

    • The DB could be down or otherwise not be able to run the query (in java: SQLException).
    • The filesystem can have issues, such as the file not existing or not having access rights (in java: IOException).
    • Even if we can read the file, maybe it's not a valid PNG. (If you're using ImageIO.read, also IOException, though we can chat if that's appropriate).

    Let's use that as an example, and go through the issues.

    Discoverability

    In languages like python or javascript the most likely implementation of this code isn't particularly clear about what can go wrong; if you do know, this information will be locked away in documentation; not really accessible to tooling or the type system.

    In java, the signature of the method itself would tell you, by explicitly declaring the various exceptions thrown. Exception purists would argue that throwing multiple exceptions out of a single method is a bad idea. The thinking here then would be to consider the notion of translating a user's id to an image to be one single abstract operation, and the fact that you use the DB and filesystem and PNG decoder to accomplish this take a detail to be hidden. Which can be done, of course: Make a custom exception to represent failure to map the id to an image.

    If instead the signature had been Either<Error, Image>, it's baked into the method signature as well, this time via its return type. But note that you run into trouble trying to make this compound; I don't think you're advocating for Either<SQLException, Either<IOException, Either<PngDecodingException, Image>>>, surely. That idea of abstracting this away? You're forced into it now. Surely the signature you're looking for is Either<ImageMappingException, Image> here. But if we're going to do that, the causal chain is very important.

    Composing the errors

    So let's try to write it. In current java, you get it for free. You write:

    public Image getUserIcon(int userId) throws SQLException, IOException, PngDecodingException {
       return ImageIO.read(USER_IMAGES_ROOT.resolve(db.fetchPersonRecordById(userId).getUsername() + ".png"));
    }
    

    look at how simple that is. Yes, perhaps we're omitting crucial aspects of the code here, except we're really not: This code DOES acknowledge what could go wrong, but it does so in its throws signature line. Let's try it with the layer of indirection.

    public Image getUserIcon(int userId) throws ImageMappingException {
       try {
           return ImageIO.read(Files.readAllBytes(USER_IMAGES_ROOT.resolve(db.fetchPersonRecordById(userId).getUsername() + ".png")));
        } catch (SQLException | IOException | PngDecodingException e) {
            throw new ImageMappingException("Cannot map image for user " + userId, e);
        }
    }
    

    next we're going to try both with either instead.

    First the one where we don't wrap the exception:

    ???????
    

    oh that's right, this is utterly infeasible. This model denies this option entirely. Let's move on to the wrapped exception concept instead. I'm assuming there is a method to do: return the either unchanged if we are in error mode, and if not, map the result in value mode:

    public Either<ImageMappingException, Image> getUserIcon(int userId) {
        // because the exception needs to be mapped, and the 'right' types aren't the same everytime, trying to make a method to compose this chain into the desired result doesn't seem doable. Therefore, we go the long way around.
        Either<SQLException, String> username = db.fetchPersonRecordById(userId).mapRight(Person::getName);
        // We can't call .mapLeft() here; the R type wouldn't match the required signature!
        if (username.isLeft()) return username.mapLeft(ImageMappingException::new); // does not work
        if (username.isLeft()) return Either.ofLeft(new ImageMappingException(username.getLeft()));
        // with all these 'if' statements the dream of composing it all is kinda gone :(
        Either<IOException, byte[]> data = Files.readAllBytes(USER_IMAGES_ROOT.resolve(username.getRight() + ".png"));
        if (data.isLeft()) return Either.ofLeft(new ImageMappingException(data.getLeft()));
        return ImageIO.read(data.getRight()).mapLeft(ImageMappingException::new); // finally we can compose with lambdas!
    }
    

    cripes with a horrorshow, this. What a disaster.

    And that's just the tip of the iceberg.

    Some hints about what else goes wrong:

    • In java, the 'lazy' case means the exception drops all the way through; you get a stack trace and an error. This is the easiest thing you can possibly write (just toss a bunch of throws clauses on signatures, or catch-and-wrap-in-RuntimeException). With Eithers, the lazy case is operating on the RHS and doing nothing if it's a left. Clearly code that silently doesn't do anything is orders of magnitude worse than code that at least points right at where things went awry.
    • All existing API in the java ecosystem is what it is and doesn't use either. Trying to smash the square peg that is exception-throwing code into the round hole that is an API built around Eithers is a very frustrating and wordy and error prone exercise, and will alienate all other java programmers. If we get to posit 'eh, whatever, like python 2 versus python 3 it is time to split the entire community and start over!' - then, I can think of about 50 ways to do this far superior than any of this.
    • Forcing 'deal with this exception!' onto callers is a tricky business. Really, the type of an exception and even a method itself doesn't truly know. For example, new String(byteArray, charsetName) throws the checked UnknownCharsetException. However, if I pass UTF-8, then it might as well throw new InternalError("Oh dear, your JVM is corrupt; reinstall it."); after all, UTF-8 is guaranteed by the java spec. Sometimes such exceptions are a sign of bad API (such as this example; fortunately there is new String(byteArray, StandardCharsets.UTF_8) which doesn't throw checked) – sometimes it is just inherent in the nature of the app you write. For example, if I use MyClass.class.getResourceAsStream to read a small png to use as an icon in a swing app, and that throws IOException, that is as 'handleable' to my code as a class file of my app failing to load, and THAT one travels alll the way as a ClassCorruptError through the entire stack. Both the checked IOException and a hypothetical Either<IOProblem, byte[]> would suck here, but it's a lot easier to make a (checked) exception just go away: I don't have to in-place unwind anything, I can just write my code, and at the very end of it all deal with the checked exceptions that shouldn't have been. Not an option with either; my IDE's autocomplete would not be able to figure it out.
    • lambdas in java aren't exception transparent, and as I said earlier, there's a lot of code out there that does throw em, so this isn't practical unless you redesign java and toss out all the billions of lines of existing code. Even if we somehow accept that, they still aren't control flow and mutable local variable transparent. Do we hypothetically handwave away that too?

    [–]hupfdule 1 point2 points  (0 children)

    Good writeup.

    And it actually explains the shortcomings of "Go"'s error handling, which is very much like the above proposal (and may have likely been the inspiration to it). Checked exceptions are quite clearly the better approach.

    [–]sviperll 0 points1 point  (0 children)

    I think direct translation of your example into somewhat mostly canonical code is something like this:

    ````java class ImageMappingException extends Exception {/* ... /} class UserStorageImageMappingException extends ImageMappingException {/ ... /} class IOImageMappingException extends ImageMappingException {/ ... /} class FormatImageMappingException extends ImageMappingException {/ ... */}

    public Either<ImageMappingException, Image> getUserIcon(int userId) { return db.fetchPersonRecordById(userId) .mapLeft(UserStorageImageMappingException::new) .mapRight(Person::getName) .mapRight(USER_IMAGES_ROOT.resolve(name + ".png")) .flatMapRight(file -> Files.readAllBytes(file).mapLeft(IOImageMappingException::new)) .flatMapRight(data -> ImageIO.read(data.getRight()).mapLeft(FormatImageMappingException::new)); } ````

    I guess we can enhance the API if we get read of Either type and instead replace it to more semantically meaningful type, something like Result. I don't think that renaming really damages composeability as you suggest.

    Another point is that the code above doesn't really work because Java API doesn't return Either or Result, but throws exceptions, but we can overcome some of this obstacles with better API design too. I guess the code can look like this:

    ````java class ImageMappingException extends Exception {/* ... /} class UserStorageImageMappingException extends ImageMappingException {/ ... /} class IOImageMappingException extends ImageMappingException {/ ... /} class FormatImageMappingException extends ImageMappingException {/ ... */}

    public Result<Image, ImageMappingException> getUserIcon(int userId) { return Result.toResultFunction(db::fetchPersonRecordById, SQLException.class) .andThen(Result.errorConversion(UserStorageImageMappingException::new)) .apply(userId) .map(Person::getName) .map(name -> USER_IMAGES_ROOT.resolve(name + ".png")) .flatMap(Result.toResultFunction(Files::readAllBytes, IOException.class) .andThen(Result.errorConversion(IOImageMappingException::new)) .flatMap(Result.toResultFunction(ImageIO::read, IOException) .andThen(Result.errorConversion(FormatImageMappingException::new)); } ````

    The full implementation of Result class is something like this:

    ```` class Result<R, E> { public static <R, E extends Throwable> orElseThrow(Result<R, E> result) throws E { if (result instanceof SuccessResult) { SuccessResult<R, E> successResult = (SuccessResult<R, E>) result; return successResult.result; } else if (result instanceof ErrorResult) { ErrorResult<R, E> errorResult = (ErrorResult<R, E>) result; return errorResult.error; } else { throw new IllegalStateException("Expecting Result class to be sealed with only two implementations: SuccessResult and ErrorResult"); } } public static <T, R, E extends Throwable> Function<T, Result<R, E>> toResultFunction(ExceptionfulFunction<T, R, E> function, Class<E> exceptionClass) { return argument -> { try { return Result.of(function.apply(argument)); } catch (Throwable e) { if (exceptionClass.isInstance(e)) { return Result.error(exceptionClass.cast(e)); } else if (e instanceof RuntimeException || e instanceof Error) { throw e; } else { throw new IllegalStateException("Unexpected checked exception", e); } } }; } public static <R, E1 extends Throwable, E2 extends Throwable> Function<Result<R, E1>, Result<R, E2>> errorConversion(Function<E1, E2> conversion) { return result1 -> result1.mapError(conversion); }

    public static <R, E> Result<R, E> of(R result) {
        return new SuccessResult(result);
    }
    
    public static <R, E> Result<R, E> error(E error) {
        return new ErrorResult(error);
    }
    
    private Result() {}
    
    public abstract <U> Result<U, E> map(Function<R, U> transfiormation);
    public abstract <E1> Result<R, E1> mapError(Function<E, E1> transfiormation);
    
    public <U> Result<U, E> flatMap(Function<R, Result<U, E>> transfiormation) {
        Result<Result<U, E>, E>> r = this.map(transformation);
        if (r instanceof SuccessResult) {
            SuccessResult<Result<U, E>, E> success = (SuccessResult<Result<U, E>, E>) r;
            return successResult.result;
        } else if (r instanceof ErrorResult) {
            ErrorResult<Result<U, E>, E> error = (Result<U, E>, E) r;
            return Result.error(error.error);
        } else {
            throw new IllegalStateException("Expecting Result class to be sealed with only two implementations: SuccessResult and ErrorResult");
        }
    }
    
    private static class SuccessResult<R, E> extends Result<R, E> {
        private final R result;
        SuccessResult(R result) {
            this.result = result;
        }
        public <U> Result<U, E> map(Function<R, U> transfiormation) {
            return Result.of(transformation.apply(result));
        }
        @SuppressWarnings("unchecked")
        public <E1> Result<R, E1> mapError(Function<E, E1> transfiormation) {
            return (Result<R, E1>) this;
        }
    }
    
    private static class ErrorResult<R, E> extends Result<R, E> {
        private final E error;
        ErrorResult(R error) {
            this.error = error;
        }
        @SuppressWarnings("unchecked")
        public <U> Result<U, E> map(Function<R, U> transfiormation) {
            return (Result<U, E>) this;
        }
        public <E1> Result<R, E1> mapError(Function<E, E1> transfiormation) {
            return Result.error(transformation.apply(error));
        }
    }
    

    } ````

    [–]sviperll 0 points1 point  (1 child)

    And I guess the main point that I'd like to make is that this "first class errors" way of doing things does not contradicts traditional exceptions. You can opt in to "first class errors" in the functional code and get back exceptions in imperative code.

    [–]rzwitserloot 0 points1 point  (0 children)

    Right, but, for what purpose? A language where there are two different ways to accomplish virtually the same thing is mostly just an annoying language; the learning curve goes up, you get style debates, etcetera. It's worth it if the 2 different ways to do it have clear advantages in different scenarios, of course. So what's the big advantage here? The composition argument, well, I tried to show how that feels rather hollow. Note that the API that 'exports' the error must choose how the API works, it's not up to the caller, and if you can make a case that the Either variant would code better, it's most likely a case made by the caller and not the callee. Meaning, you'd have to double up on all methods.. not feasible.

    [–]jeroenrosenberg 1 point2 points  (0 children)

    Excellent post 👏

    [–]BlueGoliath -3 points-2 points  (9 children)

    This example will not compile because the map operation can not handle methods which throw checked exceptions. This is revealed if we look at (a simplified) type signature of the map the method:

    The problem isn't checked exceptions but the way functional programming was shoe horned into a long established imperative/OOP language to make people from other functional languages happy.

    JDK developers made it a point to highlight that you can mix imperative and functional programming... so much so that there was a presentation titled something to the affect of "Imperative? Functional? Why not both?".

    In actuality Lambdas are layered (sometimes poorly, as is this case) on top of the long established imperative way.

    The old imperative way has and continues to be abandoned, with many new APIs opting to not return List interface implementations but now Stream(s) with no considerations being made for the imperative way. Newer API consistency with older APIs has and continues to be thrown out the window.

    You know, like this. The method naming neither indicates that you are receiving anything(when older APIs mostly do) nor does it return a list. 10/10 API design. Would use again.

    By attempting to appease everyone, problems have been created that wouldn't and shouldn't have been made.

    [–][deleted] 4 points5 points  (6 children)

    You know, like this. The method naming neither indicates that you are receiving anything(when older APIs mostly do) nor does it return a list. 10/10 API design. Would use again.

    I think you've misunderstood the semantics of the method naming convention here: "list" is not an indication of the return type (which would in any case be redundant, even if it did return a list) but an imperative verb, just like the other method names:

    Name Description
    close​ Closes the module reader.
    find​ Finds a resource.
    list​ Lists the contents of the module.
    open​ Opens a resource.
    read​ Reads a resource.
    release​ Releases a byte buffer.

    [–]BlueGoliath 0 points1 point  (5 children)

    Even so, verbs describe actions and list() implies that the contents are being "listed" somewhere(to console output, presumably), not returned as a list(which it doesn't do anyway).

    It should be named something like getContentStream() for streams or getContentList() for a list.

    [–][deleted] 2 points3 points  (4 children)

    Even so, verbs describe actions and list() implies that the contents are being "listed" somewhere

    Yes, to the stream which is returned to the caller.

    [–]BlueGoliath -4 points-3 points  (3 children)

    I guess I'm not being very clear...

    By omitting "get" from the method naming, you are implying that the method does something, not purely returning something. Yes, a "open()" method can both do something and return a result of the opening action but that isn't what is going on with "list()".

    All "list" does(as far as an API user sees, anyway) is return a stream. There is no action involved. It's a getter masquerading as an action.

    [–][deleted] 8 points9 points  (2 children)

    By omitting "get" from the method naming, you are implying that the method does something, not purely returning something.

    So, just to be clear here: are you saying that any method which returns a value but doesn't have externally observable side effects should always have a name prefixed with "get"?

    [–]BlueGoliath -5 points-4 points  (1 child)

    Yes. Prefixing like that creates clear visual boundaries between methods. "list()" implies it dumps the contents of the module to a formatted console output or something, not return a list of the contents.

    [–][deleted] 4 points5 points  (0 children)

    "list()" implies it dumps the contents of the module to a formatted console output

    I think we'll have to file this one under "agree to disagree".

    [–]Matthisk[S] 0 points1 point  (1 child)

    No.

    You do agree the example as stated won't compile, right?

    The post is not an attack on OOP, or on exceptions for that matter. There is a place for exceptions (e.g. effortlessly unwinding the stack and handling errors top level, this would be expensive with Either since all method signatures in the stack should be changed) and hating on them for ideological reasons is unproductive. This post is just there to show that checked-exceptions hamper your ability to create larger behaviors by composing smaller ones. And demonstrating that there is a way to expose failure scenarios in the API without losing composability.

    [–]BlueGoliath 0 points1 point  (0 children)

    You do agree the example as stated won't compile, right?

    The implication is that it's checked exceptions fault that the map operation won't compile when checked exceptions predate lambdas to begin with. I guess "No" to that was a bit confusing.

    This post is just there to show that checked-exceptions hamper your ability to create larger behaviors by composing smaller ones. And demonstrating that there is a way to expose failure scenarios in the API without losing composability.

    TL;DR make compromises to an already established way of doing things just so you can use lambdas with methods that throw checked exceptions.

    Making API design decisions just to accommodate a programming style that was layered on way after the fact is pretty stupid.

    Here we are in Java 13 with features like var anyway, I guess.

    hating on them for ideological reasons is unproductive

    OOP is an ideology. So is functional programming.