«

Hello, I use the following code but am not sure that it is the best or
correct way to close the stream. In particular, if url.openStream()
succeeds I have an open stream, but if any of the two other
constructors throws an exception, reader will be null and the stream
will remain open even after the finally block.

public class WhatToClose {
public static void main(String[] args) {
BufferedReader reader = null;
try{
URL url = new URL("http://www.yahoo.com/");
reader = new BufferedReader(new InputStreamReader(url.openStream
()));

// use reader here
String line = null;
while (( line = reader.readLine()) != null){
System.out.println(line);
}
} catch (Exception e) {
e.printStackTrace();
} finally {
if(reader != null){
try{
reader.close();
} catch (IOException e) {
e.printStackTrace();
}
}
}
}
}


I could allocate three refs (see below) buts that's really a PITA. How
do you do it?

public static void main(String[] args) {
InputStream is = null;
InputStreamReader isr = null;
BufferedReader br = null;
try{
URL url = new URL("http://www.yahoo.com/");
is = url.openStream();
isr = new InputStreamReader(is);
br = new BufferedReader(isr);
String line = null;
while (( line = br.readLine()) != null){
System.out.println(line);
}
} catch (Exception e) {
e.printStackTrace();
} finally {
if(br != null){
try{
br.close();
} catch (IOException e) {
e.printStackTrace();
}
}
if(isr != null){
try{
isr.close();
} catch (IOException e) {
e.printStackTrace();
}
}
if(is != null){
try{
is.close();
} catch (IOException e) {
e.printStackTrace();
}
}
}
}

Thanks Phil

Philipp wrote:
> Hello, I use the following code but am not sure that it is the best or
> correct way to close the stream. In particular, if url.openStream()
> succeeds I have an open stream, but if any of the two other
> constructors throws an exception, reader will be null and the stream
> will remain open even after the finally block.
>
> public class WhatToClose {
> public static void main(String[] args) {
> BufferedReader reader = null;
> try{
> URL url = new URL("http://www.yahoo.com/");
> reader = new BufferedReader(new InputStreamReader(url.openStream
> ()));
>
> // use reader here
> String line = null;
> while (( line = reader.readLine()) != null){
> System.out.println(line);
> }
> } catch (Exception e) {
> e.printStackTrace();
> } finally {
> if(reader != null){
> try{
> reader.close();
> } catch (IOException e) {
> e.printStackTrace();
> }
> }
> }
> }
> }
>
>
> I could allocate three refs (see below) buts that's really a PITA. How
> do you do it?
>
> public static void main(String[] args) {
> InputStream is = null;
> InputStreamReader isr = null;
> BufferedReader br = null;
> try{
> URL url = new URL("http://www.yahoo.com/");
> is = url.openStream();
> isr = new InputStreamReader(is);
> br = new BufferedReader(isr);
> String line = null;
> while (( line = br.readLine()) != null){
> System.out.println(line);
> }
> } catch (Exception e) {
> e.printStackTrace();
> } finally {
> if(br != null){
> try{
> br.close();
> } catch (IOException e) {
> e.printStackTrace();
> }
> }
> if(isr != null){
> try{
> isr.close();
> } catch (IOException e) {
> e.printStackTrace();
> }
> }
> if(is != null){
> try{
> is.close();
> } catch (IOException e) {
> e.printStackTrace();
> }
> }
> }
> }
>
> Thanks Phil

For normal streams, closing the top one is fine and will close the
others. I seem to recall some issue with URLConnection streams however.
I think there was a recent thread.

--

Knute Johnson
email s/nospam/knute2009/

--
Posted via NewsDemon.com - Premium Uncensored Newsgroup Service
------->>>>>>http://www.NewsDemon.com<<<<<<------
Unlimited Access, Anonymous Accounts, Uncensored Broadband Access

On Jul 9, 9:07*am, Philipp <djb...@gmail.com> wrote:
> Hello, I use the following code but am not sure that it is the best or
> correct way to close the stream. In particular, if url.openStream()
> succeeds I have an open stream, but if any of the two other
> constructors throws an exception, reader will be null and the stream
> will remain open even after the finally block.
>
> public class WhatToClose {
> * public static void main(String[] args) {
> * * BufferedReader reader = null;

There's another idiom that skips the 'null' assignment - see below.

> * * try{
> * * * URL url = new URL("http://www.yahoo.com/");
> * * * reader = new BufferedReader(new InputStreamReader(url.openStream
> ()));
>
> * * * // use reader here
> * * * String line = null;

No need to superfluously assign a spurious 'null' to 'line'.

> * * * while (( line = reader.readLine()) != null){
> * * * * System.out.println(line);
> * * * }
> * * } catch (Exception e) {
> * * * e.printStackTrace();
> * * } finally {
> * * * if(reader != null){
> * * * * try{
> * * * * * reader.close();

You could just trap and close the URL stream rather than the reader.

> * * * * } catch (IOException e) {
> * * * * * e.printStackTrace();
> * * * * }
> * * * }
> * * }
> * }
>
> }
>
> I could allocate three refs (see below) buts that's really a PITA. How
> do you do it?
>
> public static void main(String[] args) {
> * InputStream is = null;
> * InputStreamReader isr = null;
> * BufferedReader br = null;
>

This should not be necessary.

One way is to chain the I/O classes in separate try-catch blocks, so
that a later nullity check is unnecessary; you can use 'assert'
statements instead. The result is verbose but precise control over
what happens and how it's logged, and the safety of assertably
initialized streams and readers.

public static void main( String [] args )
{
final InputStream is;
try
{
is = url.openStream();
}
catch ( IOException exc )
{
logger.error( "Cannot open URL stream. " + exc.getMessage() );
return;
}
assert is != null;

final BufferedReader br;
try
{
br = new BufferedReader( new InputStreamReader( is ));
}
catch ( IOException exc )
{
logger.error( "Cannot open URL stream reader. " + exc.getMessage
() );
try
{
is.close();
}
catch ( IOException closex )
{
logger.error( "Cannot close URL stream. " + closex.getMessage
() );
}
return;
}
assert br != null;

try
{
for ( String line; (line = br.readLine()) != null; )
{
System.out.println(line);
}
}
catch ( IOException exc )
{
logger.error( "Cannot read URL stream reader. " + exc.getMessage
() );
}
finally
{
try
{
br.close();
}
catch ( IOException closex )
{
logger.error( "Cannot close URL stream reader. " +
closex.getMessage() );
}
}
}

--
Lew

Lew wrote:
> On Jul 9, 9:07 am, Philipp <djb...@gmail.com> wrote:
>> Hello, I use the following code but am not sure that it is the best or
>> correct way to close the stream. In particular, if url.openStream()
>> succeeds I have an open stream, but if any of the two other
>> constructors throws an exception, reader will be null and the stream
>> will remain open even after the finally block.
>>
>> public class WhatToClose {
>> public static void main(String[] args) {
>> BufferedReader reader = null;
>
> There's another idiom that skips the 'null' assignment - see below.
>
>> try{
>> URL url = new URL("http://www.yahoo.com/");
>> reader = new BufferedReader(new InputStreamReader(url.openStream
>> ()));
>>
>> // use reader here
>> String line = null;
>
> No need to superfluously assign a spurious 'null' to 'line'.
>
>> while (( line = reader.readLine()) != null){
>> System.out.println(line);
>> }
>> } catch (Exception e) {
>> e.printStackTrace();
>> } finally {
>> if(reader != null){
>> try{
>> reader.close();
>
> You could just trap and close the URL stream rather than the reader.
>
>> } catch (IOException e) {
>> e.printStackTrace();
>> }
>> }
>> }
>> }
>>
>> }
>>
>> I could allocate three refs (see below) buts that's really a PITA. How
>> do you do it?
>>
>> public static void main(String[] args) {
>> InputStream is = null;
>> InputStreamReader isr = null;
>> BufferedReader br = null;
>>
>
> This should not be necessary.
>
> One way is to chain the I/O classes in separate try-catch blocks, so
> that a later nullity check is unnecessary; you can use 'assert'
> statements instead. The result is verbose but precise control over
> what happens and how it's logged, and the safety of assertably
> initialized streams and readers.
>
> public static void main( String [] args )
> {
> final InputStream is;
> try
> {
> is = url.openStream();
> }
> catch ( IOException exc )
> {
> logger.error( "Cannot open URL stream. " + exc.getMessage() );
> return;
> }
> assert is != null;
>
> final BufferedReader br;
> try
> {
> br = new BufferedReader( new InputStreamReader( is ));
> }
> catch ( IOException exc )
> {
> logger.error( "Cannot open URL stream reader. " + exc.getMessage
> () );
> try
> {
> is.close();
> }
> catch ( IOException closex )
> {
> logger.error( "Cannot close URL stream. " + closex.getMessage
> () );
> }
> return;
> }
> assert br != null;
>
> try
> {
> for ( String line; (line = br.readLine()) != null; )
> {
> System.out.println(line);
> }
> }
> catch ( IOException exc )
> {
> logger.error( "Cannot read URL stream reader. " + exc.getMessage
> () );
> }
> finally
> {
> try
> {
> br.close();
> }
> catch ( IOException closex )
> {
> logger.error( "Cannot close URL stream reader. " +
> closex.getMessage() );
> }
> }
> }
>
> --
> Lew

Lew:

That's a pattern I've not seen before and I really like it, especially
the for loop for reading the lines. The return in the catch block is
pretty nifty too.

I am curious though if you really think that all those levels are really
necessary? The only place that can throw an I/O exception is the
URL.getInputStream() method and if it succeeds then the BufferedReader
constructor will succeed. So closing the BufferedReader will suffice.

--

Knute Johnson
email s/nospam/knute2009/

--
Posted via NewsDemon.com - Premium Uncensored Newsgroup Service
------->>>>>>http://www.NewsDemon.com<<<<<<------
Unlimited Access, Anonymous Accounts, Uncensored Broadband Access

On Thu, 9 Jul 2009, Philipp wrote:

> Hello, I use the following code but am not sure that it is the best or
> correct way to close the stream. In particular, if url.openStream()
> succeeds I have an open stream, but if any of the two other
> constructors throws an exception, reader will be null and the stream
> will remain open even after the finally block.

In this case, i'd exploit the knowledge that the upper-layer streams are
lightweight, and don't really need to be closed - if they aren't closed
but are garbage collected, that's fine. So i'd do:

InputStream urlStream = url.openStream();
try {
BufferedReader reader = new BufferedReader(new InputStreamReader(urlStream));
// use reader
}
finally {
urlStream.close();
}

In the more general case, where you might have another heavyweight stream
that needs to be explicitly closed in the stack, i'd think about keeping a
pointer to the topmost stream, so i could just close that. Since that
stream could actually be a reader, i'd make the variable a Closeable. Like
so:

InputStream urlStream = url.openStream();
Closeable topStream = urlStream;
try {
Reader isr = new InputStreamReader(urlStream)'
topStream = isr;
BufferedReader reader = new BufferedReader(isr);
topStream = reader;
// use reader here
}
finally {
topStream.close();
}

Yes, you have to write out each construction as a separate statement, and
then add a line to update topStream. But at least you only have to do the
closing once. Note that you actually only have to do the separation and
updating for each heavyweight stream in the stack - lightweight streams
can be constructed inline. Like (assuming GZIPInputStream uses libgzip
with a native context object - i could be wrong about that):

InputStream urlStream = url.openStream();
Closeable topStream = urlStream;
try {
InputStream gz = new GZIPInputStream(new BufferedInputStream(urlStream));
topStream = gz;
BufferedReader reader = new BufferedReader(new InputStreamReader(gz));
// use reader here
}
finally {
topStream.close();
}

tom

--
I have been trying to find a way of framing this but yes, a light meal is
probably preferable to a heavy one under the circumstances. -- ninebelow

Lew wrote:
>> One way is to chain the I/O classes in separate try-catch blocks, so
>> that a later nullity check is unnecessary; you can use 'assert'
>> statements instead. The result is verbose but precise control over
>> what happens and how it's logged, and the safety of assertably
>> initialized streams and readers.
>>
>> public static void main( String [] args )
>> {
>> final InputStream is;
>> try
>> {
>> is = url.openStream();
>> }
>> catch ( IOException exc )
>> {
>> logger.error( "Cannot open URL stream. " + exc.getMessage() );
>> return;
>> }
>> assert is != null;
>>
>> final BufferedReader br;
>> try
>> {
>> br = new BufferedReader( new InputStreamReader( is ));
>> }
>> catch ( IOException exc )
>> {
>> logger.error( "Cannot open URL stream reader. " + exc.getMessage
>> () );
>> try
>> {
>> is.close();
>> }
>> catch ( IOException closex )
>> {
>> logger.error( "Cannot close URL stream. " + closex.getMessage
>> () );
>> }
>> return;
>> }
>> assert br != null;
>>
>> try
>> {
>> for ( String line; (line = br.readLine()) != null; )
>> {
>> System.out.println(line);
>> }
>> }
>> catch ( IOException exc )
>> {
>> logger.error( "Cannot read URL stream reader. " + exc.getMessage
>> () );
>> }
>> finally
>> {
>> try
>> {
>> br.close();
>> }
>> catch ( IOException closex )
>> {
>> logger.error( "Cannot close URL stream reader. " +
>> closex.getMessage() );
>> }
>> }
>> }

Knute Johnson wrote:
> That's a pattern I've not seen before and I really like it, especially
> the for loop for reading the lines. The return in the catch block is
> pretty nifty too.

The return in the catch block is necessary to ensure the asserted invariant.

> I am curious though if you really think that all those levels are really
> necessary? The only place that can throw an I/O exception is the
> URL.getInputStream() method and if it succeeds then the BufferedReader
> constructor will succeed. So closing the BufferedReader will suffice.

I agree with you in this case. So the first try block would handle:

final BufferedReader br;
try
{
br = new BufferedReader( new InputStreamReader( url.openStream() ));
}
catch ( IOException exc )
{
logger.error( "Cannot open URL stream reader. " + exc.getMessage() );
return;
}
assert br != null;

The next try block would actually use 'br' and close it in its finally block.

The example I gave first was to address the OP's specific concern that the
reader assignments might fail separately from the stream retrieval, and to
illustrate the flexibility of the technique. Notice how easily the idiom
refactors to increase or decrease level of detail and granularity of control
by simply splitting out or merging try-catch blocks. The key to the idiom is
that one or more try-catch blocks establish an invariant of a valid resource
handle (the stream or reader instance), and a separate try-finally block (with
possible catch clauses) uses that guaranteed resource and disposes of it in
its finally section.

--
Lew

Lew wrote:
> Lew wrote:
>>> One way is to chain the I/O classes in separate try-catch blocks, so
>>> that a later nullity check is unnecessary; you can use 'assert'
>>> statements instead. The result is verbose but precise control over
>>> what happens and how it's logged, and the safety of assertably
>>> initialized streams and readers.
>>>
>>> public static void main( String [] args )
>>> {
>>> final InputStream is;
>>> try
>>> {
>>> is = url.openStream();
>>> }
>>> catch ( IOException exc )
>>> {
>>> logger.error( "Cannot open URL stream. " + exc.getMessage() );
>>> return;
>>> }
>>> assert is != null;
>>>
>>> final BufferedReader br;
>>> try
>>> {
>>> br = new BufferedReader( new InputStreamReader( is ));
>>> }
>>> catch ( IOException exc )
>>> {
>>> logger.error( "Cannot open URL stream reader. " + exc.getMessage
>>> () );
>>> try
>>> {
>>> is.close();
>>> }
>>> catch ( IOException closex )
>>> {
>>> logger.error( "Cannot close URL stream. " + closex.getMessage
>>> () );
>>> }
>>> return;
>>> }
>>> assert br != null;
>>>
>>> try
>>> {
>>> for ( String line; (line = br.readLine()) != null; )
>>> {
>>> System.out.println(line);
>>> }
>>> }
>>> catch ( IOException exc )
>>> {
>>> logger.error( "Cannot read URL stream reader. " + exc.getMessage
>>> () );
>>> }
>>> finally
>>> {
>>> try
>>> {
>>> br.close();
>>> }
>>> catch ( IOException closex )
>>> {
>>> logger.error( "Cannot close URL stream reader. " +
>>> closex.getMessage() );
>>> }
>>> }
>>> }
>
> Knute Johnson wrote:
>> That's a pattern I've not seen before and I really like it, especially
>> the for loop for reading the lines. The return in the catch block is
>> pretty nifty too.
>
> The return in the catch block is necessary to ensure the asserted
> invariant.
>
>> I am curious though if you really think that all those levels are
>> really necessary? The only place that can throw an I/O exception is
>> the URL.getInputStream() method and if it succeeds then the
>> BufferedReader constructor will succeed. So closing the
>> BufferedReader will suffice.
>
> I agree with you in this case. So the first try block would handle:
>
> final BufferedReader br;
> try
> {
> br = new BufferedReader( new InputStreamReader( url.openStream() ));
> }
> catch ( IOException exc )
> {
> logger.error( "Cannot open URL stream reader. " + exc.getMessage() );
> return;
> }
> assert br != null;
>
> The next try block would actually use 'br' and close it in its finally
> block.
>
> The example I gave first was to address the OP's specific concern that
> the reader assignments might fail separately from the stream retrieval,
> and to illustrate the flexibility of the technique. Notice how easily
> the idiom refactors to increase or decrease level of detail and
> granularity of control by simply splitting out or merging try-catch
> blocks. The key to the idiom is that one or more try-catch blocks
> establish an invariant of a valid resource handle (the stream or reader
> instance), and a separate try-finally block (with possible catch
> clauses) uses that guaranteed resource and disposes of it in its finally
> section.
>

Thanks Lew.

--

Knute Johnson
email s/nospam/knute2009/

--
Posted via NewsDemon.com - Premium Uncensored Newsgroup Service
------->>>>>>http://www.NewsDemon.com<<<<<<------
Unlimited Access, Anonymous Accounts, Uncensored Broadband Access