Let Unchecked.throwChecked return RuntimeException to improve control flow

[leaumar]

AS-IS

class Unchecked {
    public static void throwChecked(Throwable t) {
        SeqUtils.sneakyThrow(t);
    }
}

public Object foo() {
    if (condition) {
        return bar();
    } else {
        Unchecked.throwChecked(new Exception());
        return null;
    }
}

TO-BE

class Unchecked {
    public static <T> T throwChecked(Throwable t) {
        SeqUtils.sneakyThrow(t);
        return null;
    }
}

public Object foo() {
    if (condition) {
        return bar();
    } else {
        return Unchecked.throwChecked(new Exception());
    }
}

Versions:

• jOOλ: 0.9.12
• Java: 1.8.0.91

[leaumar]

Alternative: return RuntimeException instead of T so we can throw Unchecked.throwChecked(ex); instead of return Unchecked.throwChecked(ex), just to shut the compiler up about control flow not ending. Might work better with analysis tools that would complain about a return value being null in context of @Nonnull and such.

[lukaseder]

I can definitely see the problem, and the solution looks clever, but also a bit surprising. I'm not that much of a fan of the return variant. It also only works for reference types, not for primitive types. We could, however, pretend to return a RuntimeException and then you could write something like:

throw Unchecked.unchecked(new TimeoutException());

Will think about this. Of course, in the meantime, the most appropriate workaround is something like this:

public Object foo() {
    if (condition) {
        return bar();
    } else {
        Unchecked.throwChecked(new Exception());
        throw new IllegalStateException();
    }
}

[lukaseder]

There are essentially two options to solving this:

1. Adapt the existing method (incompatibly)
2. Introduce a new method for this purpose

I'm currently favouring the latter, if we can find an appropriate name for it.

[leaumar]

throw Unchecked.throwBack throw Unchecked.returnThrow/throwReturn throw Unchecked.feignReturning

fake, false, out, pretend, escalate, reciprocate, echo, mirror, break, elevate, subvert, overcharge, dissent, hide, disguise...

Just throwing some words that come to mind for inspiration

[lukaseder]

Hah, nice ones :). Let's look at it more thoroughly.

1. The method will throw the argument exception. So, terms like "return" won't do the trick.
2. The return value is there only to allow for it to be used in a dummy throw statement. Thus, terms like "reciprocate", "echo", or "mirror" won't do the trick.
3. Since using this new method with a throw statement is the expected usage, repeating the word throw in the method name might be confusing (despite 1.)
4. Terms like "feign" or "fake" might indicate some more complex trickery, or even swallowing exceptions. It would make the programmer feel uneasy about calling the method. That doesn't do it.
5. We're still throwing a checked exception as if it were an unchecked exception, so the term should be somewhat similar to the existing throwChecked()
6. The method can be called standalone without using throw

All of these bullets indicate, if anything at all, that we're about to do a hack :)

[billoneil]

Few more ideas. throw Unchecked.asUnchecked(new Exception()); throw Unchecked.cast(new Exception()); throw Unchecked.coerce(new Exception()); throw Unchecked.whyCantCheckedExceptionsBeACompilerFlag(new Exception());

[lukaseder]

While I totally agree with the criticism implied in the last name, none of these method names suggesed by @billoneil convey the fact that the method actually throws the argument exception. They all seem to imply that they simply return a transformed exception to be thrown. That's exactly the kind of misleading naming I'd like to avoid.

[leaumar]

throwThenReturn? But it won't even actually return anything because it will throw first, so the name would need to be throwAndFeignReturn or feignReturnButActuallyThrow or something.

I feel like expressing every detail of the functionality as a single method name will lead us to InitializingBeanFactoryProviderContextConsumingPropertyEvaluatingBeanSource situations... Developers should (and mostly do) read at least the basic description that comes with methods provided by libraries, so personally I think the method should be given a concise, simple name that hints at throwing and fake returning, and if someone actually wants to use it they'll read the details in the doc.

[leaumar]

Oh, and I would like to bring up the initial return (T) null idea again. There's a usecase for it:

Map<Class<X>, Function<X, T>> mappers = new HashMap<>();
mappers.put(A.class, a -> foo(a));
// sorry, we don't support Bs here
mappers.put(B.class, b -> Unchecked.whatchaMeCallit(new NotSupportedException()));

Making it return (T) null allows our method on Unchecked to disguise itself as a program component, similar to an overridden method throwing UnsupportOperationException. Making it return an exception will exclude it from being pluggable into systems that expect certain object types as return value.

I was gonna say I don't see how primitives are an issue with this, but then I figured out that assigning a T to a primitive triggers a warning about autoboxing and null values...

Honestly though, why not both? Let people use the T variant where they want to plug our whatchaMeCallIt into their program as a component (excluding situations with primitives, if they care about the warning), and let them use the variant returning a RuntimeException where they are able to add the throw keyword. That should cover almost everything they might need.

[lukaseder]

personally I think the method should be given a concise, simple name that hints at throwing and fake returning, and if someone actually wants to use it they'll read the details in the doc.

Exactly. Thus far, I haven't seen this name yet...

Oh, and I would like to bring up the initial return (T) null idea again. There's a usecase for it:

I see, that's a nice use-case. Yet, the reference type vs primitive type discussion still holds, here... Well, it could be generic and we could hope that auto-boxing will be performed (but never executed).

[leaumar]

that auto-boxing will be performed (but never executed).

It can't ever be executed because an exception will be thrown first... And since how many versions ago does java autobox now?

[lukaseder]

It can't ever be executed because an exception will be thrown first...

Yes, that's what I said :)

[leaumar]

I think Unchecked.throwAndReturn is about as good as it gets. It's short, says exactly what it will do, and has no redundant wording (throwAndReturnUnchecked is a bit too verbose if it's called with its Unchecked. prefix).

Maybe throwReturningNull for the generic variant

[lukaseder]

Noh, sorry, that's not going to do it. We're not in a hurry to add something like this to the library...

[asm0dey]

Ermm… Isn't sneakyThrow commonly accepted name?

[lukaseder]

@asm0dey Sure, maybe. But that name still leads to the same confusion about the ambiguity introduced by a method that throws and returns at the same time.

[leaumar]

Honestly, I don't think anyone will be confused because of that :/ And if someone is, they can see at a glance what it does when they open the source via their IDE, or they can see the javadoc that any decent IDE will also just inline...

[asm0dey]

@lukaseder sometimes convention is not perfect but is well-known, accepted and thus suitable for similar situations. Sorry for invterventing into your dialogue.

[lukaseder]

Sometimes convention is not perfect but is well-known, accepted and thus suitable for similar situations

1. Internally, there's already a sneakyThrow method, probably for the same reasons (found the name on google, and perhaps in lombok), but the public name is now throwChecked... Not too keen on renaming that
2. I'm sure you have data to prove the fact that the name is well-known, and accepted :)

Sorry for invterventing into your dialogue.

No worries. I do that all the time, myself :)

[lukaseder]

@TheRealMarnes I respectfully disagree. Having funky names in an API is something a maintainer spends a lot of time discussing with many people. Surely, this particular case is trivial and we could say "Just this one time" (akin to https://xkcd.com/292/). But after 10 years of doing public OSS APIs, I've come to regret quite a few decisions, so I've become rather restrictive on public API evolution and naming things.

However, maybe, I'll simply revert my opinion here: https://github.com/jOOQ/jOOL/issues/324#issuecomment-348160452 and implement the backwards-incompatible solution that you initially proposed.

[asm0dey]

Sneaky throw is well known in very thin community of developers who know what the hell that is - articles and lombok call it this way, I think all we have heard about this. And it's pretty unnatural to call something tryReturnButThrow it something like that :) Because we know it's just sneaky throw )))

чт, 1 мар. 2018 г., 13:41 Lukas Eder notifications@github.com:

@TheRealMarnes https://github.com/therealmarnes I respectfully disagree. Having funky names in an API is something a maintainer spends a lot of time discussing with many people. Surely, this particular case is trivial and we could say "Just this one time" (akin to https://xkcd.com/292/). But after 10 years of doing public OSS APIs, I've come to regret quite a few decisions, so I've become rather restrictive on public API evolution and naming things.

However, maybe, I'll simply revert my opinion here: #324 (comment) https://github.com/jOOQ/jOOL/issues/324#issuecomment-348160452 and implement the backwards-incompatible solution that you initially proposed.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jOOQ/jOOL/issues/324#issuecomment-369551742, or mute the thread https://github.com/notifications/unsubscribe-auth/ABPLghimYeeuI5alH7cHOyP05x2l-UbAks5tZ9BigaJpZM4Qvx27 .

[ben-manes]

Guava used to have Throwables.propagate(t) which used this pattern, so you could end off with throw Throwables.propagate(t). It was nice, but also deprecated in favor of an explicit throw. They can provide more context, to explain why it became an anti-pattern within the Google codebase.