uschindler commented on pull request #643: URL: https://github.com/apache/lucene/pull/643#issuecomment-1030807558
> This isn't exactly right - it is legal bytecode but it isn't exactly playing well with the language. Hi @dweiss: Let's not discuss this here, the whole thing does not apply to this PR. My recommendation was to add another functional interface to IOUtils (we already created them there). I would never design my APIs in a way like Java streams API was done, so having a separate functional interface for IO actions is a good idea. The above tweet is more about working around problems with Java Streams API. We know for sure that Java Streams API internally handles exception in a correct way (using finally blocks), so the code example you posted does not apply there. To me, checked exceptions are a misconception of Java, but you can start flamewars about that. Another misconception was to let MethodHandles.invoke throw Throwable. Thats another case where sneaky rethoring must be done to have nice behaviour. But there it is no problem because you left Java Type System already. So inside methods that call MethodHandles it is recommended and perfectly fine to use rethrow, as long as you make sure the caller form public API gets correct exceptions (means you sneaky rethrow all excepions, but you declare the method to have the checked exceptions you know of). BTW, Java inte rnally also rethrows like this in the lamda bootstraps (I rewrote it for Elasticsearch Painless language). In general: Every cast is a misuse of the Java language, and so is this one. What we do here is just casting the exception type. So it is a misuse of type system (like every cast is), but it is not a misuse of the language. Its perfectly legal to do it. If you try out my above Sneaky class and look (with eclipse) at the types introduced for the generics type arguments, it is funny how intelligent the Java compiler is: If you have a functional interface throwing `E extends Exception` and the code does not throw any checked exception, Javac inserts `RuntimeException` into the type argument. I already talked too much, my problem with Sneaky is that if you use such type casting, the Java Compiler won't allow you to catch checked Exceptions anymore, that's the worst thing that happens - but Sneaky works grell if you make sure that the method itsself calling your erased Java streams is redeclaring `throws IOException`. The try...catch posted above is.... bad code anyways. Better use finally blocks to ensure something is executed for sure. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
