different try-finally approach

[Pitch]

In java it is common to check if the resource is null in finally
statement, before closing it:

public void doSomething() throws MyResourceException
{
MyResource res = null;
try
{
res = new MyResource();
// do something
}
finally
{
if (res != null) res.close()
}
}


But in some other languages this is the preferred way:

public void doSomething() throws MyResourceException
{
MyResource res = new MyResource();
try
{
// do something
}
finally
{
res.close()
}
}


The first example ensures nothing more than the second one, right? If
the constructor throws an exception the "res" is null, so it can't be
closed anyways.

So, my question is - why the extra coding in java? Am I missing
something?

 

[Bill McCleary]

In java it is common to check if the resource is null in finally
statement, before closing it:

public void doSomething() throws MyResourceException
{
MyResource res = null;
try
{
res = new MyResource();
// do something
}
finally
{
if (res != null) res.close()
}
}


But in some other languages this is the preferred way:

public void doSomething() throws MyResourceException
{
MyResource res = new MyResource();
try
{
// do something
}
finally
{
res.close()
}
}


The first example ensures nothing more than the second one, right? If
the constructor throws an exception the "res" is null, so it can't be
closed anyways.

So, my question is - why the extra coding in java? Am I missing
something?

If there's more than one resource, the latter form to be exception safe
has to be nested several times, each one in the previous one's try
block. The former form, on the other hand, avoids all this nesting and
deep indentation becoming:

public void doSomething() throws MyResourceException
{
MyResource foo = null;
AnotherResource bar = null;
Quuxulator baz = null;
try
{
foo = new MyResource();
bar = ...;
// do something
baz = ...;
// do some more
}
finally
{
if (foo != null) foo.close()
if (bar != null) bar.close()
if (baz != null) baz.close()
}
}

 

[Mike Schilling]

If there's more than one resource, the latter form to be exception
safe has to be nested several times, each one in the previous one's
try block. The former form, on the other hand, avoids all this
nesting and deep indentation becoming:

public void doSomething() throws MyResourceException
{
MyResource foo = null;
AnotherResource bar = null;
Quuxulator baz = null;
try
{
foo = new MyResource();
bar = ...;
// do something
baz = ...;
// do some more
}
finally
{
if (foo != null) foo.close()
if (bar != null) bar.close()
if (baz != null) baz.close()
}
}

This is not quite good enough when (as with streams or readers),
"close()" can itself throw.

 

[Roedy Green]

ut in some other languages this is the preferred way:

public void doSomething() throws MyResourceException
{
MyResource res = new MyResource();
try
{
// do something
}
finally
{
res.close()
}
}

That approach implies new MyResource() cannot fail. The Java way
handles the failure.

--
Roedy Green Canadian Mind Products
http://mindprod.com

"Patriotism is fierce as a fever, pitiless as the grave, blind as a stone, and as irrational as a headless hen."
~ Ambrose Bierce (born: 1842-06-24 died: 1914 at age: 71)

 

[Andreas Leitgeb]

The first example ensures nothing more than the second one, right? If
the constructor throws an exception the "res" is null, so it can't be
closed anyways.
So, my question is - why the extra coding in java? Am I missing
something?

Yes, in that particular case those two snippets are equivalent.

Out of gut-feeling I'd still choose the first one (with "new" inside the
try-block), for reasons not covered by your outset.

When I see your code, I think: "some other day I will want to *catch*
those errors thrown from the 'new MyRessource()' line",
or "some day I *will* have more ressources, as in Bill's followup".

How furtunate such fortune-telling really is, only future will show

 

[Tom Anderson]

In java it is common to check if the resource is null in finally
statement, before closing it:

public void doSomething() throws MyResourceException
{
MyResource res = null;
try
{
res = new MyResource();
// do something
}
finally
{
if (res != null) res.close()
}
}

But in some other languages this is the preferred way:

public void doSomething() throws MyResourceException
{
MyResource res = new MyResource();
try
{
// do something
}
finally
{
res.close()
}
}


The first example ensures nothing more than the second one, right? If
the constructor throws an exception the "res" is null, so it can't be
closed anyways.

So, my question is - why the extra coding in java? Am I missing
something?

I'm not aware that the former is 'preferred' in java. I certainly strongly
prefer the latter. Indeed, i consider the former an error, and i'll fix it
when i see it in code.

tom

 

[Lew]

In java [sic] it is common to check if the resource is null in finally
statement, before closing it:

public void doSomething() throws MyResourceException
{
MyResource res = null;
try
{
res = new MyResource();
// do something
}
finally
{
if (res != null) res.close()
}
}

Ugly.

But in some other languages this is the preferred way:

public void doSomething() throws MyResourceException
{
MyResource res = new MyResource();
try
{
// do something
}
finally
{
res.close()
}
}

This is also doable in Java.

I suppose, but it is different.

the constructor throws an exception the "res" is null, so it can't be
closed anyways.

So, my question is - why the extra coding in java [sic]? Am I missing
something?

Both examples you show are valid in Java.

I prefer neither one. I eliminate the redundant assignment to 'null' and use
try-catch on the resource allocation, with a followup assertion, then a
separate try-finally on the resource usage.

I'm not aware that the former is 'preferred' in java [sic]. I certainly
strongly prefer the latter. Indeed, i [sic] consider the former an error, and
i'll [sic] fix it when i [sic] see it in code.

I'm with tom, except I don't like merely rethrowing exceptions as the second
example shows. It is better to catch, log and handle the exception
immediately. If there's a rethrow, it'll be to a higher-level exception.

I prefer to put the resource allocation in a try-catch with return on failure,
and the resource usage in a separate try-catch-finally, with an assertion
between them that the resource pointer is not null.

There is no "extra" coding in any of the three alternatives. Resource
allocation must be guarded against failure; code that deals with potential
failure is not "extra". However, one could regard the assignment to 'null' as
superfluous if one takes the view that a failed resource allocation should
result in no assignment whatsoever to the resource pointer.

 

[Pitch]

If there's more than one resource, the latter form to be exception safe
has to be nested several times, each one in the previous one's try
block. The former form, on the other hand, avoids all this nesting and
deep indentation becoming:

public void doSomething() throws MyResourceException
{
MyResource foo = null;
AnotherResource bar = null;
Quuxulator baz = null;
try
{
foo = new MyResource();
bar = ...;
// do something
baz = ...;
// do some more
}
finally
{
if (foo != null) foo.close()
if (bar != null) bar.close()
if (baz != null) baz.close()
}
}

Well, I see this is handy but I don't think this a good programming
practice. This way several blocks of processing are bundled together.

As well, close() itself is not very safe, as Mike suggested.

I would do this:

public void doSomething() throws MyResourceException
{
MyResource foo = new MyResource();
try
{
// ...
processAnotherResource();
}
finally
{
foo.close()
}
}

public void processAnotherResource() throws MyResourceException
{
AnotherResource bar = new AnotherResource ();
try
{
// ...
}
finally
{
bar.close()
}
}

 

[Arved Sandstrom]

I'm not aware that the former is 'preferred' in java. I certainly
strongly prefer the latter. Indeed, i consider the former an error, and
i'll fix it when i see it in code.

tom

It's possible that a code snippet here or there gave the OP a mistaken
impression. For example, the exception handling discussion in the Java
tutorial does have something like

finally {
if (out != null) {
out.close();
} else {
...
}
}

although there are catch blocks (the example method is handling all
exceptions, not rethrowing any), and "out" is a PrintWriter that could
in fact be null in the finally block (the PrintWriter ctor, IOW, is
wrapping a FileWriter ctor which is what could throw an exception).
However, the discussion is split out among a number of pages and hasty
scanning of the material might lead to a wrong conclusion.

Although if you're Googling then the very first link you run across
using the keywords "Java finally" is
http://www.javapractices.com/topic/TopicAction.do?Id=25, where Style 1
has it that if you're rethrowing the exception, that you follow the
style of the second example given by the OP.

In a nutshell it's not clear to me why the OP thought that example 1 was
the "common" or "preferred" way, unless it was careless reading.
Understanding what can throw exceptions, where you wish to handle
exceptions, and what resources need to be disposed of, always leads you
to a simple and correct way of writing your exception handling code,
IMHO. I have no idea why it's such a mysterious topic.

AHS

 

[Pitch]

That approach implies new MyResource() cannot fail. The Java way
handles the failure.

Actually it doesn't.

In both situations constructor is allowed to fail. But in "java way" if
constructor fails, the close() statement is never called, so it's pretty
much the same thing, right?

I guess the bottom line is wheather you'll use internal catch or an
external one. I prefer the external ones, or even dividing the code
between methods.

 

[Pitch]

Yes, in that particular case those two snippets are equivalent.

Out of gut-feeling I'd still choose the first one (with "new" inside the
try-block), for reasons not covered by your outset.

When I see your code, I think: "some other day I will want to *catch*
those errors thrown from the 'new MyRessource()' line",
or "some day I *will* have more ressources, as in Bill's followup".

OK, but in such cases why not break up the code?

Look at this example:

public void copyFiles(String inFile, String outFile)
throws IOException
{
FileOutputStream outStream = null;
FileInputStream inStream = null;

try
{
FileOutputStream outStream = new FileOutputStream(outFile);
FileInputStream inStream = new FileInputStream(inFile);
// copying data...
}
finally
{
if (inStram != null) inStram.close();
if (outStram != null) outStram.close();
}
}


Now, look at this one:

public void copyFiles(String inFile, String outFile)
throws IOException
{
FileInputStream inStream = new FileInputStream(inFile);
try
{
copyToFile(inStream, outFile);
}
finally
{
inStram.close();
}
}

private void copyToFile(InputStream inStream, String outFile)
throws IOException
{
FileOutputStream outStream = new FileOutputStream(inFile);
try
{
// copying code...
}
finally
{
outStream.close();
}
}



Maybe you guys don't like writing as many methods?

 

[Pitch]

Tom Anderson wrote:

In a nutshell it's not clear to me why the OP thought that example 1 was
the "common" or "preferred" way, unless it was careless reading.
Understanding what can throw exceptions, where you wish to handle
exceptions, and what resources need to be disposed of, always leads you
to a simple and correct way of writing your exception handling code,
IMHO. I have no idea why it's such a mysterious topic.

Ok, I'm in programming business like 20 years. I've worked with try-
blocks in Delphi and C#. I'm working with java for about 6 years now.
I've always used the style I've learned from Delphi an C#, which is
without the null assigments.

Now, I've seen hundreds of examples with the null assigment. All my
colleagues in several companies used that style. You actually cannot
find a different example on the whole Sun's site!!

So I was wondering does Sun knows something that I don't??

Perhaps it has something to do with multithreading? Deadlocks??

I've found similar discussion here:

<http://stackoverflow.com/questions/463029/initializing-disposable-
resources-outside-or-inside-try-finally>

 

[Stefan Ram]

Maybe you guys don't like writing as many methods?

See my example code for accessing SQL sources using
nested attempts separated into multiple methods so
as to avoid deep nesting and logged close-exceptions
on:

http://www.purl.org/stefan_ram/pub/java_msaccess_de

, albeit with sorrounding text in German.

The imagined purpose of the program is to print
a table with headings from an SQL data base.

I am always looking to improve this, so let me know
if you see any bugs or potential problems.

 

[Arved Sandstrom]

Ok, I'm in programming business like 20 years. I've worked with try-
blocks in Delphi and C#. I'm working with java for about 6 years now.
I've always used the style I've learned from Delphi an C#, which is
without the null assigments.

Now, I've seen hundreds of examples with the null assigment. All my
colleagues in several companies used that style. You actually cannot
find a different example on the whole Sun's site!!

So I was wondering does Sun knows something that I don't??

Perhaps it has something to do with multithreading? Deadlocks??

I've found similar discussion here:

<http://stackoverflow.com/questions/463029/initializing-disposable-
resources-outside-or-inside-try-finally>

Whether you have a declaration with a null assignment is - to me - a
consequence of the structuring of your code (i.e. scope), and where the
exception(s) is/are going to be handled, and perhaps the examples we
have been using are too simple to illustrate that. IOW, if we had
significant amounts of "do something", and possibly other exceptions,
then it could make sense to have the earlier declaration with the
required null assignment.

In any case, now that I'm really looking at the two simple examples, let
me ask for clarification on one point. I, and presumably other posters,
have been assuming that it's the MyResource ctor that's throwing the
exception, based on your phrasing in your first post. Can the "do
something" also throw MyResourceException? What about the close() method
on the MyResource object? If nothing else throws this exception - only
the ctor - and the doSomething() method rethrows anyhow, why do we have
try {} and finally {} at all?

AHS

 

[Pitch]

In any case, now that I'm really looking at the two simple examples, let
me ask for clarification on one point. I, and presumably other posters,
have been assuming that it's the MyResource ctor that's throwing the
exception, based on your phrasing in your first post. Can the "do
something" also throw MyResourceException? What about the close() method
on the MyResource object? If nothing else throws this exception - only
the ctor - and the doSomething() method rethrows anyhow, why do we have
try {} and finally {} at all?

Well, yes, I assumed any part of code could throw MyResourceException,
even close().

if an exception occurs in constructor it's ok, the resource is not
acquired, nothing is done and caller can handle it.

If it occurs in try-finally (or any onther checked or unchecked
exception, like RuntimeException) the code can try to close it. This is
the main reason for having try-finally

If it occurs in close() the caller knows there are unclosed resources
and can handle it.

 

[Pitch]

See my example code for accessing SQL sources using
nested attempts separated into multiple methods so
as to avoid deep nesting and logged close-exceptions
on:

http://www.purl.org/stefan_ram/pub/java_msaccess_de

, albeit with sorrounding text in German.

The imagined purpose of the program is to print
a table with headings from an SQL data base.

I am always looking to improve this, so let me know
if you see any bugs or potential problems.

First of all, why declare everything with final? I know it is usefull if
you need access from an inner class but since those are not class
members, but local variables, maybe you don't need that!? Anyway, I'm no
expert.

Next, you catch Exception when closing. I woud make a simple method just
for the sake of readability

private static void safeCloseConnection(Connection conn, StringBuilder
log)
{
try
{
conn.close();
}
catch (Throwable th) // not just Exception
{
log.add(ex);
}
}


Also for closing the statement I would also make a
safeCloseStatement(Statement s, ...) and
safeCloseResultSet(ResultSet, ...)

I'm a radability freak.

Other than that I would only suggest that whenever you have an if-
statment, you must think what happens if it's not true. You should add
an "else" for every "if" and at least put some information in the log.

And one other thing, those are all static methods and you must transfer
every argument in each call. Maybe you should consider making it all
instance members?

 

[Stefan Ram]

First of all, why declare everything with final?

To convey my intention not to modify this variable.

Next, you catch Exception when closing. I woud make a
simple method just for the sake of readability

It does not neccesarily have to improve readability and
maintainability, because additional methods do not
only have benefits, but also a price.

catch (Throwable th) // not just Exception

Ok, this really will suppress any throw, but I am
not sure whether I want to catch and log an
OutOfMemoryError there.

Other than that I would only suggest that whenever
you have an if- statment, you must think what happens
if it's not true. You should add an "else" for every
"if" and at least put some information in the log.

Yes, I will check this one!

And one other thing, those are all static methods and
you must transfer every argument in each call. Maybe
you should consider making it all instance members?

Then, my special style of always using the same
method name just different parameter tuplets to
indicate which resources already are obtained
might not work anymore - and I like this style.

If it would not be for that special case here,
I would agree with you.

Thanks for the review!

 

[Bill McCleary]

This is not quite good enough when (as with streams or readers),
"close()" can itself throw.

In actual practice I tend to have a utility method like this somewhere
in a project that uses lots of I/O:

public static void close (Closeable c) {
if (c != null) {
try {
c.close();
} catch (IOException e) {
// Ignore.
}
}
}

which gets rid of the (IMHO useless) close() exceptions, and also allows
nulls (which it ignores).

Then it's

finally {
IOUtils.close(foo);
IOUtils.close(bar);
IOUtils.close(baz);
}

or similarly.

As for the objection that more than one job is being done, maybe, maybe
not. In many cases a single conceptual job involves more than one
stream. For example, copying something out of one file into another
involves working simultaneously with an InputStream and an OutputStream.
These might be wrapped in Reader and Writer, or records from a single
input file might be signal-splittered to several distinct output files,
one for records of type X, one for records of type Y, etc., so there'd
be one InputStream and multiple OutputStreams. Or several input files
are appended -- one of each stream type again, but with the InputStream
being reassigned periodically.

 

[Pitch]

To convey my intention not to modify this variable.


It does not neccesarily have to improve readability and
maintainability, because additional methods do not
only have benefits, but also a price.

Ok. I'd like to think readability is more important than performance.

Thanks for the review!

Np.

 

[Pitch]

In actual practice I tend to have a utility method like this somewhere
in a project that uses lots of I/O:

public static void close (Closeable c) {
if (c != null) {
try {
c.close();
} catch (IOException e) {
// Ignore.
}
}
}

which gets rid of the (IMHO useless) close() exceptions, and also allows
nulls (which it ignores).

Then it's

finally {
IOUtils.close(foo);
IOUtils.close(bar);
IOUtils.close(baz);
}

Nice.


As for the objection that more than one job is being done, maybe, maybe
not. In many cases a single conceptual job involves more than one
stream. For example, copying something out of one file into another
involves working simultaneously with an InputStream and an OutputStream.
These might be wrapped in Reader and Writer, or records from a single
input file might be signal-splittered to several distinct output files,
one for records of type X, one for records of type Y, etc., so there'd
be one InputStream and multiple OutputStreams. Or several input files
are appended -- one of each stream type again, but with the InputStream
being reassigned periodically.

I see and I agree there could be scenarios where the neatest approach
would be to check for nulls. I belive we are closing the subject.
Apparently there is no "mystical" reason to go either this or that way.