Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > comp.lang.java.programmer > #26252 > unrolled thread

unnecessary code in Oracle example?

Back to article view | Back to comp.lang.java.programmer

Contents

  unnecessary code in Oracle example? Jim Janney <jjanney@shell.xmission.com> - 2013-10-09 10:38 -0600
    Re: unnecessary code in Oracle example? Daniel Pitts <newsgroup.nospam@virtualinfinity.net> - 2013-10-09 11:19 -0700
      Re: unnecessary code in Oracle example? ram@zedat.fu-berlin.de (Stefan Ram) - 2013-10-09 19:04 +0000
      Re: unnecessary code in Oracle example? Jim Janney <jjanney@shell.xmission.com> - 2013-10-09 13:08 -0600
        Re: unnecessary code in Oracle example? markspace <markspace@nospam.nospam> - 2013-10-09 12:27 -0700
          Re: unnecessary code in Oracle example? Jim Janney <jjanney@shell.xmission.com> - 2013-10-09 14:21 -0600
            Re: unnecessary code in Oracle example? markspace <markspace@nospam.nospam> - 2013-10-09 14:56 -0700
              Re: unnecessary code in Oracle example? Jim Janney <jjanney@shell.xmission.com> - 2013-10-09 20:56 -0600
                Re: unnecessary code in Oracle example? Arivald <NOSPAMarivald@interia.pl> - 2013-10-10 06:42 +0200
                  Re: unnecessary code in Oracle example? Sebastian <news@seyweiler.dyndns.org> - 2013-10-10 07:47 +0200
                    Re: unnecessary code in Oracle example? Arivald <arivald_@AT_interia_DOT_.pl> - 2013-10-10 10:26 +0200
                      Re: unnecessary code in Oracle example? Jim Janney <jjanney@shell.xmission.com> - 2013-10-10 08:47 -0600
      Re: unnecessary code in Oracle example? Robert Klemme <shortcutter@googlemail.com> - 2013-10-09 21:11 +0200
        Re: unnecessary code in Oracle example? cuzdav@gmail.com - 2013-10-10 08:49 -0700

#26252 — unnecessary code in Oracle example?

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

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

[toc] | [next] | [standalone]

#26255

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

[toc] | [prev] | [next] | [standalone]

#26256

Daniel Pitts <newsgroup.nospam@virtualinfinity.net> 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.

[toc] | [prev] | [next] | [standalone]

#26257

Daniel Pitts <newsgroup.nospam@virtualinfinity.net> 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

[toc] | [prev] | [next] | [standalone]

#26259

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

[toc] | [prev] | [next] | [standalone]

#26260

markspace <markspace@nospam.nospam> 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

[toc] | [prev] | [next] | [standalone]

#26263

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.

[toc] | [prev] | [next] | [standalone]

#26269

markspace <markspace@nospam.nospam> 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

[toc] | [prev] | [next] | [standalone]

#26271

W dniu 2013-10-10 04:56, Jim Janney pisze:
> markspace <markspace@nospam.nospam> 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

[toc] | [prev] | [next] | [standalone]

#26272

Am 10.10.2013 06:42, schrieb Arivald:
> W dniu 2013-10-10 04:56, Jim Janney pisze:
>> markspace <markspace@nospam.nospam> 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?
>
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

[toc] | [prev] | [next] | [standalone]

#26277

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:
>>> markspace <markspace@nospam.nospam> 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?
>>
> 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


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.

-- 
Arivald

[toc] | [prev] | [next] | [standalone]

#26291

Arivald <arivald_@AT_interia_DOT_.pl> writes:

> 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:
>>>> markspace <markspace@nospam.nospam> 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?
>>>
>> 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
>
>
> 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.

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.

-- 
Jim Janney

[toc] | [prev] | [next] | [standalone]

#26258

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

[toc] | [prev] | [next] | [standalone]

#26292

> 
> 
> > This is one reason why C++ forbids destructors from throwing.
> 
> Does it?
> 

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

[toc] | [prev] | [standalone]

Back to top | Article view | comp.lang.java.programmer

csiph-web