unnecessary code in Oracle example?

J

Jim Janney

I'm reading up on Java 7's try-with-resource statement and looking at
the tutorial at

http://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html

which includes the following code:
static String readFirstLineFromFileWithFinallyBlock(String path)
throws IOException {
BufferedReader br = new BufferedReader(new FileReader(path));
try {
return br.readLine();
} finally {
if (br != null) br.close();
}
}
Click to expand...

As a matter of habit, I always write that pattern as

static String readFirstLineFromFileWithFinallyBlock(String path)
throws IOException {
BufferedReader br = new BufferedReader(new FileReader(path));
try {
return br.readLine();
} finally {
br.close();
}
}
Click to expand...

on the theory that if you reach that point br can never be null, so the
test is both redundant and confusing. On the other hand, I might be
wrong. Is there a reason to test for null in the finally block?
 
D

Daniel Pitts

I'm reading up on Java 7's try-with-resource statement and looking at
the tutorial at

http://docs.oracle.com/javase/tutorial/essential/exceptions/tryResourceClose.html

which includes the following code:


As a matter of habit, I always write that pattern as



on the theory that if you reach that point br can never be null, so the
test is both redundant and confusing. On the other hand, I might be
wrong. Is there a reason to test for null in the finally block?
Click to expand...
You are correct for this case, the other case comes from a block which
uses multiple resources.

MyResource r1 = null;
MyResource r2 = null;

try {
r1 = openResource1();
r2 = openResource2();
} finally {
if (r1 != null) r1.close();
if (r2 != null) r2.close();
}

Of course, some close() methods can throw, which could break this idiom
too. This is one reason why C++ forbids destructors from throwing.
 
S

Stefan Ram

Daniel Pitts said:
MyResource r1 = null;
MyResource r2 = null;
try {
r1 = openResource1();
r2 = openResource2();
} finally {
if (r1 != null) r1.close();
if (r2 != null) r2.close();
}
Click to expand...

Which also might be written as follows
(execution would start at r() below,
code was never syntax-checked nor tested).

void use
( final MyResource r1,
final MyResource r2 )
{ /* insert the code that needs r1 und r2 here */ }

void r2
( final MyResource r1,
final MyResource r2 )
{ try
{ use( r1, r2 ); }
finally
{ r2.close(); }}

void r1( final MyResource r1 )
{ try
{ r2( r1, openRessource2() ); }
catch( final OpenResource2Exception exception )
{ /* add possible exception handling here */ }
finally
{ r1.close(); }}}

void r()
{ try
{ r1( openRessource1() ); }
catch( final OpenResource1Exception exception )
{ /* add possible exception handling here */ }}

The function calls are a trick that I have invented to
bypass the need for non-final null-initialized variables
when the reference of a resource just obtained is to be
stored in a variable.

I also do not have to compare the resources with null before
closing them, since the functions with the close() calls are
only called when the corresponding open calls did not throw.
 
J

Jim Janney

Daniel Pitts said:
You are correct for this case, the other case comes from a block which
uses multiple resources.

MyResource r1 = null;
MyResource r2 = null;

try {
r1 = openResource1();
r2 = openResource2();
} finally {
if (r1 != null) r1.close();
if (r2 != null) r2.close();
}

Of course, some close() methods can throw, which could break this
idiom too. This is one reason why C++ forbids destructors from
throwing.
Click to expand...

Thanks. But that looks like a good idiom to avoid. I prefer

MyResource r1 = openResource();
try {
MyResource r2 = openResource2();
try {
// do something
} finally {
r2.close();
}
} finally {
r1.close();
}

Not as pretty syntactically, perhaps, but much clearer semantically.
 
R

Robert Klemme

If you declare the variable "final" then there is no reason. The way
you showed it "br" can actually become null.
You are correct for this case, the other case comes from a block which
uses multiple resources.

MyResource r1 = null;
MyResource r2 = null;

try {
r1 = openResource1();
r2 = openResource2();
} finally {
if (r1 != null) r1.close();
if (r2 != null) r2.close();
}

Of course, some close() methods can throw, which could break this idiom
too.
Click to expand...

The proper (i.e. robust) way to do it would be to spend one try finally
block per resource. That may not be the prettiest of idioms because of
the indentation but it's robust.
This is one reason why C++ forbids destructors from throwing.
Click to expand...

Does it?

$ cat -n x.cxx
1
2
3 #include <iostream>
4
5 class Ex {
6 };
7
8 class Foo {
9 public:
10 ~Foo() throw (Ex) {
11 std::cout << "destructor" << std::endl;
12 throw Ex();
13 }
14 };
15
16 int main() {
17 try {
18 Foo f;
19 std::cout << "foo" << std::endl;
20 }
21 catch (Ex e) {
22 std::cout << "caught" << std::endl;
23 }
24
25 return 0;
26 }
27
robert@fussel:~$ g++ x.cxx && ./a.out
foo
destructor
caught

Cheers

robert
 
M

markspace

Thanks. But that looks like a good idiom to avoid. I prefer

MyResource r1 = openResource();
try {
MyResource r2 = openResource2();
try {
// do something
} finally {
r2.close();
}
} finally {
r1.close();
}

Not as pretty syntactically, perhaps, but much clearer semantically.
Click to expand...

I don't like either one. Why not take advantage of the fact that
resources are specified to be closed in the opposite order that they are
declared in a try-with-resources?

The following demonstrates this by closing the named streams in the
order C-B-A:


package quicktest;

import java.io.IOException;
import java.io.InputStream;

public class TryResourcesTest
{
public static void main( String[] args ) throws Exception
{
try(
MyTestStream a = new MyTestStream( "A" );
MyTestStream b = new MyTestStream( "B" );
MyTestStream c = new MyTestStream( "C" )
) {
System.out.println( a.read()+b.read()+c.read() );
}
}
}

class MyTestStream extends InputStream {

private final String name;

public MyTestStream( String name ) {
this.name = name;
}

@Override
public int read() throws IOException {
return 42;
}

@Override
public void close() {
System.out.println( getClass().getSimpleName()+
":"+name+" closed." );
}
}
 
J

Jim Janney

markspace said:
Thanks. But that looks like a good idiom to avoid. I prefer

MyResource r1 = openResource();
try {
MyResource r2 = openResource2();
try {
// do something
} finally {
r2.close();
}
} finally {
r1.close();
}

Not as pretty syntactically, perhaps, but much clearer semantically.
Click to expand...

I don't like either one. Why not take advantage of the fact that
resources are specified to be closed in the opposite order that they
are declared in a try-with-resources?

The following demonstrates this by closing the named streams in the
order C-B-A:


package quicktest;

import java.io.IOException;
import java.io.InputStream;

public class TryResourcesTest
{
public static void main( String[] args ) throws Exception
{
try(
MyTestStream a = new MyTestStream( "A" );
MyTestStream b = new MyTestStream( "B" );
MyTestStream c = new MyTestStream( "C" )
) {
System.out.println( a.read()+b.read()+c.read() );
}
}
}

class MyTestStream extends InputStream {

private final String name;

public MyTestStream( String name ) {
this.name = name;
}

@Override
public int read() throws IOException {
return 42;
}

@Override
public void close() {
System.out.println( getClass().getSimpleName()+
":"+name+" closed." );
}
}
Click to expand...

Yes, in Java 7 you can have clean syntax and clean semantics. Looks
like a win all around.
 
M

markspace

Yes, in Java 7 you can have clean syntax and clean semantics. Looks
like a win all around.
Click to expand...

For Java 5+, with varargs you could do (not compiled):

public class MyIoUtils {

public static void closeAll( Closeable ... toClose ) {
for( Closeable closeMe : toClose )
try {
if( closeMe != null ) closeMe.close();
} catch( IOException ex ) {
Logger.getLogger( "MyIoUtils" ).log( ex );
}
}
}

and get a somewhat cleaned up try catch

OutPutStream io1 = null;
BufferedStream bs = null;
try {
io1 = new OutputStream( ...
bs = new BufferedStream( io1, ...
// do stuff
}
finally {
MyIoUtils.closeAll( bs, io1 );
}

Mostly I don't care for deep nesting, I think it reads poorly, and I
think it can also be hard to trace out for a programmer reading the
source. Giving the concept of closing a lot of objects a name like
"closeAll" promotes literate programming, imo.
 
J

Jim Janney

markspace said:
For Java 5+, with varargs you could do (not compiled):

public class MyIoUtils {

public static void closeAll( Closeable ... toClose ) {
for( Closeable closeMe : toClose )
try {
if( closeMe != null ) closeMe.close();
} catch( IOException ex ) {
Logger.getLogger( "MyIoUtils" ).log( ex );
}
}
}

and get a somewhat cleaned up try catch

OutPutStream io1 = null;
BufferedStream bs = null;
try {
io1 = new OutputStream( ...
bs = new BufferedStream( io1, ...
// do stuff
}
finally {
MyIoUtils.closeAll( bs, io1 );
}

Mostly I don't care for deep nesting, I think it reads poorly, and I
think it can also be hard to trace out for a programmer reading the
source. Giving the concept of closing a lot of objects a name like
"closeAll" promotes literate programming, imo.
Click to expand...

Mmm... you're complicating the code with extra assignments and tests,
just to avoid one layer of nesting. I like the version with two
try-finally blocks, partly because the lexical structure reflects what's
happening in the code: two independent cleanups. But the Java 7 version
is better.
 
A

Arivald

W dniu 2013-10-10 04:56, Jim Janney pisze:
markspace said:
For Java 5+, with varargs you could do (not compiled):
Click to expand...
[...]

Mmm... you're complicating the code with extra assignments and tests,
just to avoid one layer of nesting. I like the version with two
try-finally blocks, partly because the lexical structure reflects what's
happening in the code: two independent cleanups. But the Java 7 version
is better.
Click to expand...

I wonder how Java 7 version (try-with-resources) handle exceptions...
For example, if all 3 streams throw exception from close(), will it
guarantee to close all of them? And how to handle such exceptions?
 
S

Sebastian

Am 10.10.2013 06:42, schrieb Arivald:
W dniu 2013-10-10 04:56, Jim Janney pisze:
markspace said:
On 10/9/2013 1:21 PM, Jim Janney wrote:
Yes, in Java 7 you can have clean syntax and clean semantics. Looks
like a win all around.


For Java 5+, with varargs you could do (not compiled):
Click to expand...
[...]

Mmm... you're complicating the code with extra assignments and tests,
just to avoid one layer of nesting. I like the version with two
try-finally blocks, partly because the lexical structure reflects what's
happening in the code: two independent cleanups. But the Java 7 version
is better.
Click to expand...

I wonder how Java 7 version (try-with-resources) handle exceptions...
For example, if all 3 streams throw exception from close(), will it
guarantee to close all of them? And how to handle such exceptions?
Click to expand...
Why don't you modify markspace's example to see for yourself, e. g.

@Override
public void close() throws IOException {
System.out.println( "Trying to close "
+ getClass().getSimpleName()+":"+name );
throw new IOException("CloseError " + name);
}

Then read what the Javadocs have to say about suppressed exceptions.

-- Sebastian
 
A

Arivald

W dniu 2013-10-10 07:47, Sebastian pisze:
Am 10.10.2013 06:42, schrieb Arivald:
W dniu 2013-10-10 04:56, Jim Janney pisze:
On 10/9/2013 1:21 PM, Jim Janney wrote:
Yes, in Java 7 you can have clean syntax and clean semantics. Looks
like a win all around.


For Java 5+, with varargs you could do (not compiled): [...]

Mmm... you're complicating the code with extra assignments and tests,
just to avoid one layer of nesting. I like the version with two
try-finally blocks, partly because the lexical structure reflects what's
happening in the code: two independent cleanups. But the Java 7 version
is better.
Click to expand...

I wonder how Java 7 version (try-with-resources) handle exceptions...
For example, if all 3 streams throw exception from close(), will it
guarantee to close all of them? And how to handle such exceptions?
Click to expand...
Why don't you modify markspace's example to see for yourself, e. g.

@Override
public void close() throws IOException {
System.out.println( "Trying to close "
+ getClass().getSimpleName()+":"+name );
throw new IOException("CloseError " + name);
}

Then read what the Javadocs have to say about suppressed exceptions.

-- Sebastian
Click to expand...


I read docs about suppressed exceptions. And of course I can run test.
But still I wonder how it should behave, if there is more than one
close-able resource, two of them raise exceptions, and there was no
exception in main block (so no suppression).

Can anyone point me to relevant doc? Google is not very helpful.
 
J

Jim Janney

Arivald said:
W dniu 2013-10-10 07:47, Sebastian pisze:
Am 10.10.2013 06:42, schrieb Arivald:
W dniu 2013-10-10 04:56, Jim Janney pisze:

On 10/9/2013 1:21 PM, Jim Janney wrote:
Yes, in Java 7 you can have clean syntax and clean semantics. Looks
like a win all around.


For Java 5+, with varargs you could do (not compiled):
[...]

Mmm... you're complicating the code with extra assignments and tests,
just to avoid one layer of nesting. I like the version with two
try-finally blocks, partly because the lexical structure reflects what's
happening in the code: two independent cleanups. But the Java 7 version
is better.


I wonder how Java 7 version (try-with-resources) handle exceptions...
For example, if all 3 streams throw exception from close(), will it
guarantee to close all of them? And how to handle such exceptions?
Click to expand...
Why don't you modify markspace's example to see for yourself, e. g.

@Override
public void close() throws IOException {
System.out.println( "Trying to close "
+ getClass().getSimpleName()+":"+name );
throw new IOException("CloseError " + name);
}

Then read what the Javadocs have to say about suppressed exceptions.

-- Sebastian
Click to expand...


I read docs about suppressed exceptions. And of course I can run
test. But still I wonder how it should behave, if there is more than
one close-able resource, two of them raise exceptions, and there was
no exception in main block (so no suppression).

Can anyone point me to relevant doc? Google is not very helpful.
Click to expand...

http://blog.eyallupu.com/2013/03/try-catch-resource-and.html

Something to think about when reworking old code, if anyone has time for
that.
 
C

cuzdav

Does it?
Click to expand...

True, it isn't forbidden directly, but the "rule" is a clear
consequence of the way the language works, where throwing
destructors cause most programs to misbehave.

The standard does explicitly warn that it's a bad idea:

15.2.3: The process of calling destructors for automatic objects
constructed on the path from a try block to a throw-expression is
called "stack unwinding." If a destructor called during stack
unwinding exits with an exception, std::terminate is called
(15.5.1). [ Note: So destructors should generally catch exceptions and
not let them propagate out of the destructor. —end note ]

But this is a java forum...
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Similar Threads

Revised Question on File Processing 2
Inserting Multiple Lines from Console 14
Closing decorated streams 6
counting the repeated values of a certain row in array 0
NumberFormatException:please help me......!!! 4
How to sort a CSV file with merge sort JAVA 7
Linux net command showing incorrect output if executing from java. 0
finally block in Exception 4

Members online

Total: 78 (members: 2, guests: 76)
Robots: 458

Forum statistics

Threads
473,995
Messages
2,570,233
Members
46,820
Latest member
GilbertoA5

Latest Threads

Top