Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > Java > unnecessary code in Oracle example?

Reply
Thread Tools

unnecessary code in Oracle example?

 
 
Jim Janney
Guest
Posts: n/a
 
      10-09-2013
I'm reading up on Java 7's try-with-resource statement and looking at
the tutorial at

http://docs.oracle.com/javase/tutori...urceClose.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();
> }
> }


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();
> }
> }


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?

--
Jim Janney
 
Reply With Quote
 
 
 
 
Daniel Pitts
Guest
Posts: n/a
 
      10-09-2013
On 10/9/13 9:38 AM, Jim Janney wrote:
> I'm reading up on Java 7's try-with-resource statement and looking at
> the tutorial at
>
> http://docs.oracle.com/javase/tutori...urceClose.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();
>> }
>> }

>
> 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();
>> }
>> }

>
> 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?

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.
 
Reply With Quote
 
 
 
 
Stefan Ram
Guest
Posts: n/a
 
      10-09-2013
Daniel Pitts <(E-Mail Removed)> writes:
>MyResource r1 = null;
>MyResource r2 = null;
>try {
> r1 = openResource1();
> r2 = openResource2();
>} finally {
> if (r1 != null) r1.close();
> if (r2 != null) r2.close();
>}


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.

 
Reply With Quote
 
Jim Janney
Guest
Posts: n/a
 
      10-09-2013
Daniel Pitts <(E-Mail Removed)> writes:

> 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.


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.

--
Jim Janney
 
Reply With Quote
 
Robert Klemme
Guest
Posts: n/a
 
      10-09-2013
On 10/09/2013 08:19 PM, Daniel Pitts wrote:
> On 10/9/13 9:38 AM, Jim Janney wrote:
>> I'm reading up on Java 7's try-with-resource statement and looking at
>> the tutorial at
>>
>> http://docs.oracle.com/javase/tutori...urceClose.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();
>>> }
>>> }

>>
>> 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();
>>> }
>>> }

>>
>> 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?


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.


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.


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
 
Reply With Quote
 
markspace
Guest
Posts: n/a
 
      10-09-2013
On 10/9/2013 12:08 PM, Jim Janney wrote:
>
> 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.
>


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." );
}
}

 
Reply With Quote
 
Jim Janney
Guest
Posts: n/a
 
      10-09-2013
markspace <(E-Mail Removed)> writes:

> On 10/9/2013 12:08 PM, Jim Janney wrote:
>>
>> 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.
>>

>
> 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." );
> }
> }


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

--
Jim Janney
 
Reply With Quote
 
markspace
Guest
Posts: n/a
 
      10-09-2013
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):

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.



 
Reply With Quote
 
Jim Janney
Guest
Posts: n/a
 
      10-10-2013
markspace <(E-Mail Removed)> writes:

> 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):
>
> 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.


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.

--
Jim Janney
 
Reply With Quote
 
Arivald
Guest
Posts: n/a
 
      10-10-2013
W dniu 2013-10-10 04:56, Jim Janney pisze:
> markspace <(E-Mail Removed)> writes:
>
>> 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?

--
Arivald

 
Reply With Quote
 
 
 
Reply

Thread Tools

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
install_driver(Oracle) failed: Can't load 'C:/Perl/site/lib/auto/DBD/Oracle/Oracle.dll' for module DBD::Oracle: load_file:The specified procedure could not be found at C:/Perl/lib/DynaLoader.pm line 230. Feyruz Perl Misc 4 10-14-2005 06:47 PM
how to prevent unnecessary table resizing Dominik Jain HTML 9 08-04-2005 06:02 PM
Common pro*c code for both Oracle and non-Oracle environments Golan C Programming 1 04-05-2005 06:34 PM
XSLT: avoiding unnecessary ns declaration in HTML output Steffen Beyer XML 3 11-02-2004 12:53 PM
Unnecessary Network trafic generated between only two comp out of =?Utf-8?B?bWlja3l0ZWpzaW5naEB5YWhvby5jb20=?= Wireless Networking 1 10-29-2004 09:49 AM



Advertisments