cactoos
cactoos copied to clipboard
Should we stop using checked exceptions?
I noticed the pain it can be to use all the interfaces like Text or Scalar that throws Exception.
You have to wrap them most of the time in IoCheckedXXX or UncheckedText.
In the end it feels like all this gymnastic is defeating the whole purpose of having cached exceptions. And the only places where we catches exceptions in cactoos are… where we catch the superclass Exception!
I just read this: https://pragmaticobjects.com/chapters/001_checked_exceptions.html and there is in the comments a discussion about cactoos.
I wonder if we should not:
- introduce our own
UncheckedException extends RuntimeException - use it everywhere it is necessary to re-throw a checked exception.
@llorllale/z please, pay attention to this issue
@victornoel
sigh it's a relevant discussion but I'm parking this one for the moment being...
Let's first get #913 and #915 - and their aftermaths! - out of the way.
@llorllale :+1: seems good to me!
This is a big one, we can already start discussing it with interested parties maybe (@amihaiemil? @skapral? :)
@victornoel the main purpose I wrote this post is to prove that there is nothing good in checked exceptions for "Elegant Objects" approach. And Cactoos IMO is very representative example of that. These UncheckedScalar stuff and similar classes which wrap checked exceptions to unchecked is just a burden, and a cheat for overriding the principles.
So, I'd go further and just:
- Remove all
throws Exceptionsfrom interfaces. - Remove all classes with
Checked/Uncheckedin their names. - Wrap all checked exceptions to unchecked immediately in method, when one need to deal with stuff which throws them.
It would:
- reduce the amount of classes in cactoos, leaving only those which do real business
- simplify cactoos usage in general
- improve polymorphism
@yegor256 @skapral @llorllale @victornoel @mdbs99 @paulodamaso Ok, I was convinced. @skapral is right. Let's remove checked exceptions because it's more a hindrance than a help. See also yergor256/takes#920.
I am using cactoos for over a year now. After the first months I had the same opinion as @victornoel . But the more and more native cactoos code I wrote, the more it all made sense. Basically, checked exceptions exist. And they exist for a reason. Applications process data. This data comes from a network, database or file system. All these APIs throw checked exceptions. With using interfaces I can wrap objects that access the data sources at runtime on actually being called. I don't have to write copy-code that creates stupid data carrying "simple" objects. And with SolidScalar of cactoos I could easily cache data once loaded. The moments where I had to wrap my scalars and texts into the unchecked became less and less.
So I am a big fan of the checked exceptions in cactoos. I never understood the fear of exceptions. It's a thing you always have to have in mind. And it tends to slip through your fingers when all those methods you call do not throw anything ever. But they do and kill your processes where you didn't expect it.
A good example is the Java thread pool API. You hand the executor a Runnable that has to be executed periodically. And of course the Runnable does not declare "throws Exception". Because an exception at this point kills everything. The executor just won't trigger another execution of the Runnable object once there was an exception. But when your business code that pumps data from a file system into a network never declares to throw an exception it is very likely you will write a Runnable implementation that doesn't care to handle exceptions at this crucial point. I'd even say that every developer with less than 3 years practice will do it wrong when not being forced to think about it by checked exceptions.
My point of view is that the checked exception must be the default, so that you stumble against those "walls" where you simply can not propagate exceptions anymore.
A last comment: Without the checked exception, the Scalar class is equivalent to Java Supplier and all it adds to plain Java are the higher caching classes.
I just read the article of @skapral mentioned in the issue text. It's a good article but it didn't change my mind. Maybe Java should invent a "no exceptions behind this point" outline. Or Java should force you to also hand over an exception handler with your Runnable object. But right now you can only mark 90% of your code unsafe to make the traps visible where you must not throw an exception at all.
@martin-robo Check this thread. In general, I'd recommend also to go through in-comment discussions there, because topics you've raised were already touched there in some way or the other.
it is very likely you will write a Runnable implementation that doesn't care to handle exceptions at this crucial point.
Handling exceptions is typically wrong way to work with them. It's called flow control via exceptions, and I'm on Yegor's side there. The only reason to catch exception should be to collect some more info from the context and rethrow further. In the end, no matter which kind of exception you have, it'll hit the bottom anyway.
Instead, at the crucial points of application logic, developer should make it in a way when exceptions aren't possible.
So I am a big fan of the checked exceptions in cactoos. I never understood the fear of exceptions. It's a thing you always have to have in mind
If it's thing that must be always kept in mind, what is the reason to keep it declared?
But right now you can only mark 90% of your code unsafe to make the traps visible where you must not throw an exception at all.
You may try leaving a trap where you must not throw an exception at all. Like FracSum in the example: it is safe and there is not supposed to be any checked exceptions, right? But you'll quickly catch yourself into that trap once you tried to reuse safe object with unsafe. It'll cause you all those artificial useless constructs.
Why don't we discuss it further in comments to the post? I don't think people here will be pleased if we raise long discussions here.
Hi @skapral and thank you for the quick reply. The thread you linked is a good one. Describing the client side of the whole thing made sense to me.
Although I still think there are moments when exception handling is essential and may be forgotten when all the untrusted code is not being marked. But these moments are very rare. Therefor you won't see concequences often and it is not a very important topic. But I am also one that marks really everything final that is final. So maybe it's just my thing to write a lot of additional details around my code.
For now I'm a bit lost frustrated because I already wrote thousands of lines of code with cactoos and now a core principle is being turned upside down. Of course this one is on me. I will have to remove everything, write my own elegant library to cope in the meantime and wait for 1.0 of cactoos. I really like the rising elegant objects community and hope it will be giving Java a better API than Oracle is now creating for years with it's static hacking magic. Best regards.
@martin-robo
For now I'm a bit lost frustrated because I already wrote thousands of lines of code with cactoos and now a core principle is being turned upside down
I'm sorry that it impacts your solutions. But you don't need to remove everything. The only thing you are to remove would be throws Exceptions, and adding try-catch wrapping blocks in places where you currently use UncheckedScalars or similar stuff. The rest of the composition remains as-is.
If you don't want to remove throws Exceptions, you may reproduce all removed artifacts in your project. Maybe even outline them as add-in package.
@victornoel I think we need return to this discussion and I vote to remove all exceptions from interfaces. It will be drive to a simpler design and more composable object system.
@skapral @fabriciofx I kind of agree.
Do you think we really need our own UncheckedException or actually the ones available in the java lib (such asIllegalStateException) are good enough for most of the case when we have to catch a checked exception?
@victornoel Theoretically type of exception can matter for rather cosmetic scenarios of how the stack trace will eventually be shown to developers. Like, custom exception's name can give some hint on the cause of the issue, or it can provide a place to collect more information from the context of where exception is thrown from (the exception's attributes), but that's it. From handling perspective --- type shouldn't matter (or at least it should be as rare case as possible), or else it will be nasty flow control.
@victornoel I agree with @skapral. And I've a question: is it in your plans start to remove checked exceptions soon?
@fabriciofx I'm still thinking about it, it feels like a good idea and I have argued for it in the past of course :)
@yegor256 WDYT as PO?
@victornoel let's remove it. @yegor256 hasn't time to decide it. We're on our own.
@fabriciofx @skapral @llorllale @yegor256 so let's remove checked exception :)
The reasoning is the following:
- checking everything is the same as checking nothing: in the end, the effort is in the hands of the implementors
- when all cactoos interfaces throw
Exception, usually implementors just let the exception bubble up (see the current code base as an example) - it is good practice to add some context to exceptions that bubbles up
So:
- so it's best to have cactoos interfaces not throwing exception, to push their implementors toward catching from class throwing checked exception and adding context instead of letting them bubble up
- for all the other cases, e.g., when using a class not throwing checked exception, it's the same as point 2., whatever we choose, so we should make a conscious effort in cactoos to add context even when no checked exceptions are thrown
I will soon create a new clean ticket for this, to try to do this in multiple steps, this is what I have in mind, please chip in to give your opinion about how to tackle this:
- we should remove check exception but replace them in each and every class with try/catch to add some context to any exception being thrown when relevant
- we should decide between introducing a dedicated runtime exception for cactoos or instead the existing
IllegalStateExceptionandIllegalArgumentException- from my experience, the latter will be completely ok and enough to cover all cases
@fabriciofx @skapral @llorllale @yegor256 some more questions that are bothering me on that matter: if we remove the exception, shouldn't we switch to using java.lang.Function, BiFunction, Consumer, BiConsumer and Supplier to replace Func, BiFunc, Proc, BiProc and Scalar?