Cleaner exception handling

[elharo]

No description provided.

[rmannibucau]

@elharo hmm, why is it cleaner ? Difference is not much except your flavor adds bytecode for a case which will never happen so basically you add dead code from my window, any real case worth a unit test and this change?

[elharo]

1. It removes a formal variable and keeps everything within the try block. This is how try blocks were designed.
2. If it turns out that the IOException is not actually impossible, then it's signaled instead of being swallowed so the bug can be addressed.

[rmannibucau]

1. well this is discussable even if I agree on the intent, but the code pattern you do use is also wrong for try blocks cause it misses the flush (in terms of "cleaness", luckily this does nothing there but if you assume that you can also assume there will never be any IOException) which is a well know issue of the return within the try so looks worse than before to me
2. not a fan of coding for an hypothesis, there is literally not a single line throwing that exception in the related class so it is impossible and other exceptions will blow up as expected

so tempted to think this PR is not cleaner so really +-0 since it will literally just add dead code and not change the behavior

[elharo]

try-with-resources handles the flush. That's what this block was originally trying to do with the ignored variable.

[rmannibucau]

try-with-resources handles the flush.

Please run

class Foo {
    @Test
    void run() throws IOException {
        try (final var writer = new StringWriter() {
            @Override
            public void flush() {
                throw new IllegalStateException("this will not happen");
            }
        }) {
            writer.write("test");
        }
    }
}

That's what this block was originally trying to do with the ignored variable.

No, the current code works cause we call toString after close which does handle flush, no more with your version

[github-actions]

This pull request is stale because it has been waiting for feedback for 60 days. Remove the stale label or comment on this PR, or it will be automatically closed in 30 days.

[github-actions]

@elharo Please assign appropriate label to PR according to the type of change.

[rmannibucau]

hmm, why this broken pattern got merged? we shouldnt encourage to read before closing IO IMHO