Am 01/08/17 um 03:18 schrieb Tibor Digana:
> Currently I have two clone o master. The HEAD and 4d36 and I see the
> difference on the file system.
>
> I think this is wrong:
> reader.close()
> reader = null;
> finally {
The only situation for reader != null here is when the try block has not
succeeded to the final statement (reader = null) because there has been
an exception. So the IOUtils.close(reader) only performs a real close
call, if there already has been an exception in the try block. That
exception is what needs to propagate. That's the reason the IOUtil.close
method just ignores any exception. It's misused nearly everyhwere.
> IOUtils.close(reader);
> }
> I know that IOUtils swallows exception on close().
> I agree that better would be
>
> method () throws IOException
> <read>
> }
> finally
> {
> reader.close();
> }
The 'finally' clause is just there to clean up any resources when the
try block has thrown an exception. So the exception to propagate is the
one from the try block. If the exception from the finally block is
propagated, the causing exception from the try block disappears. You get
a stacktrace about a failing close on a resource which produced an
exception on usage you cannot see anywhere. So for the example above,
the close call in the finally block also is executed when there has been
an exception in the try block. It's most likely that the resource
failing in the try block will again fail in the finally block. That will
propagate the exception from the finally clause but not the original
exception from the try block. So maybe the try block would produce an
IOException with "Permission denied" you never see because you get an
IOException with "Close failed" hard to track to the original failure.
> Most probably read/write operation would fail on before closing the stream,
> but I would rather not to use IOUtil.close() since it swallows the
> exception.
It's just a static utility method to prevent you from having to write:
finally
{
try
{
if(resource != null)
{
resource.close();
}
}
catch(IOException e)
{
// Suppressed.
}
}
The resource != null check is the important thing and setting it to null
as the last statement in the try block to indicate successful
completion. That's why I stopped using all kinds of WhateverUtil.close
and instead write it as shown above. I will now write that comment like
"Suppressed, so that the exception thrown in the try block will be
propagated." instead of just "Suppressed." in the future to make it even
clearer the exception needs to be ignored.
>
> In case of PrintWriter it was just more simple to use it because of not
> needed additional three objects:
> FileOutputStream
> OutputStreamWriter
> BufferedWriter
>
> I still do not understand why Oracle does not have IOException in method
> signatures of PrintWriter but hopefully they introduced checkError().
> For user it does not matter if the exception was originated from write() or
> synthetic one. The important is that the plugin fails on fatal error
> without hanging and with meaningful message.
> I think we can do both:
> + compact code
> + IOException as well
> We can add our utility class which closes PrintWriter, calls checkError()
> and throws IOException. WDYT?
I am just heading for not ignoring any exceptions thrown silently. It's
that what makes it hard to find out about issues you could easily spot
if you have a stacktrace instead of - well - nothing but some unexpected
behaviour.
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
