The close() operation for streams

S

Stefan Ram

When creating and using a file input stream,
what is the most recommendable boilerplate?

Or, what should be taught in a tutorial?

This first possibility?

{ final java.io.FileInputStream fileInputStream
= new java.io.FileInputStream( sourceFile );
/* use the fileInputStream */ }

Or this second possibility?

{ final java.io.FileInputStream fileInputStream
= new java.io.FileInputStream( sourceFile );
try
{ /* use the fileInputStream */ }
finally
{ fileInputStream.close(); }}

~~

The following code does not contain »finally« for
simplification, but is it good, unneccessary, or harmful to
write all these close()s in this order?

final java.io.FileInputStream fileInputStream
= new java.io.FileInputStream( sourceFile );

final java.io.InputStreamReader inputStreamReader
= new java.io.InputStreamReader( fileInputStream, "UTF-8" );

final java.io.BufferedReader bufferedReader
= new java.io.BufferedReader( inputStreamReader );

/* ... use bufferedReader here ... */

bufferedReader.close();
inputStreamReader.close();
fileInputStream.close();
 
K

Knute Johnson

Stefan said:
When creating and using a file input stream,
what is the most recommendable boilerplate?

Or, what should be taught in a tutorial?

This first possibility?

{ final java.io.FileInputStream fileInputStream
= new java.io.FileInputStream( sourceFile );
/* use the fileInputStream */ }

Or this second possibility?

{ final java.io.FileInputStream fileInputStream
= new java.io.FileInputStream( sourceFile );
try
{ /* use the fileInputStream */ }
finally
{ fileInputStream.close(); }}

~~

The following code does not contain »finally« for
simplification, but is it good, unneccessary, or harmful to
write all these close()s in this order?

final java.io.FileInputStream fileInputStream
= new java.io.FileInputStream( sourceFile );

final java.io.InputStreamReader inputStreamReader
= new java.io.InputStreamReader( fileInputStream, "UTF-8" );

final java.io.BufferedReader bufferedReader
= new java.io.BufferedReader( inputStreamReader );

/* ... use bufferedReader here ... */

bufferedReader.close();
inputStreamReader.close();
fileInputStream.close();
Click to expand...

BufferedReader br = null;
try {
br = new BufferedReader(
new InputStreamReader(
new FileInputStream("file")));

// read data
} catch (IOException ioe) {
// deal with exception
} finally {
if (br != null)
try {
br.close();
} catch (IOException ioe) {
// deal with close exception
}
}

The null test is needed in case there was an exception thrown and br was
never set to the BufferedReader. Closing the outer stream or reader
closes the inner(s).
 
J

Jim

When creating and using a file input stream,
what is the most recommendable boilerplate?

Or, what should be taught in a tutorial?

This first possibility?

{ final java.io.FileInputStream fileInputStream
= new java.io.FileInputStream( sourceFile );
/* use the fileInputStream */ }

Or this second possibility?

{ final java.io.FileInputStream fileInputStream
= new java.io.FileInputStream( sourceFile );
try
{ /* use the fileInputStream */ }
finally
{ fileInputStream.close(); }}
Click to expand...
<snip>

I've used the following with no problems...

InputStream is;
try {
is = new FileInputStream(sourceFile);
// Use the stream
} catch(IOException ioe) {
// Do something constructive with the exception
} finally {
if(is != null) {
try {
is.close();
} catch(IOException ioe) {
// log it or something
}
}

Jim
 
R

Roedy Green

finally
{ fileInputStream.close(); }}
Click to expand...

I think you need
if (fileInputStream != null )
{
fileInputStream.close();
}

then you can put the open in the try block too.

Otherwise you need two try blocks. I have a strong aversion to any
extra nesting.
 
S

Stefan Ram

Roedy Green said:
Otherwise you need two try blocks. I have a strong aversion to any
extra nesting.
Click to expand...

Thanks for all answers!

Here is my current tutorial code. I accept some uneccessary
close()-calls (e.g., closing the input stream reader that might
already have been closed by closing the buffered reader.)

I have avoided visible nesting of two try clauses by splitting
the code into several methods.

The hypothetical problem is to print the first line of a text
file given by its path:

public class Main
{
public static void main( final java.lang.String[] args )
throws
java.io.FileNotFoundException,
java.io.UnsupportedEncodingException,
java.io.IOException
{ final java.lang.String sourceFileName = "C:/example.txt";
printFirstLineOf( sourceFileName ); }

public static void printFirstLineOf( final java.lang.String sourceFileName )
throws
java.io.FileNotFoundException,
java.io.UnsupportedEncodingException,
java.io.IOException
{ final java.io.File sourceFile = new java.io.File( sourceFileName );
printFirstLineOf( sourceFile ); }

public static void printFirstLineOf( final java.io.File sourceFile )
throws
java.io.FileNotFoundException,
java.io.UnsupportedEncodingException,
java.io.IOException
{ /* throws java.io.FileNotFoundException */
final java.io.FileInputStream fileInputStream
= new java.io.FileInputStream( sourceFile );
try
{ printFirstLineOf( fileInputStream ); }
finally
{ fileInputStream.close(); }}

public static void printFirstLineOf( final java.io.FileInputStream fileInputStream )
throws
java.io.UnsupportedEncodingException,
java.io.IOException
{ /* throws java.io.UnsupportedEncodingException */
final java.io.InputStreamReader inputStreamReader
= new java.io.InputStreamReader( fileInputStream, "UTF-8" );
try
{ printFirstLineOf( inputStreamReader ); }
finally
{ inputStreamReader.close(); }}

public static void printFirstLineOf( final java.io.InputStreamReader inputStreamReader )
throws java.io.IOException
{ final java.io.BufferedReader bufferedReader
= new java.io.BufferedReader( inputStreamReader );
try
{ printFirstLineOf( bufferedReader ); }
finally
{ bufferedReader.close(); }}

public static void printFirstLineOf( final java.io.BufferedReader bufferedReader )
throws java.io.IOException
{ /* throws java.io.IOException */
final java.lang.String line = bufferedReader.readLine();
java.lang.System.out.println( line ); }}
 
E

EJP

Stefan said:
Roedy Green said:
Otherwise you need two try blocks. I have a strong aversion to any
extra nesting.
Click to expand...

Thanks for all answers!

Here is my current tutorial code. I accept some uneccessary
close()-calls (e.g., closing the input stream reader that might
already have been closed by closing the buffered reader.)

I have avoided visible nesting of two try clauses by splitting
the code into several methods.

The hypothetical problem is to print the first line of a text
file given by its path:

public class Main
{
public static void main( final java.lang.String[] args )
throws
java.io.FileNotFoundException,
java.io.UnsupportedEncodingException,
java.io.IOException
{ final java.lang.String sourceFileName = "C:/example.txt";
printFirstLineOf( sourceFileName ); }

public static void printFirstLineOf( final java.lang.String sourceFileName )
throws
java.io.FileNotFoundException,
java.io.UnsupportedEncodingException,
java.io.IOException
{ final java.io.File sourceFile = new java.io.File( sourceFileName );
printFirstLineOf( sourceFile ); }

public static void printFirstLineOf( final java.io.File sourceFile )
throws
java.io.FileNotFoundException,
java.io.UnsupportedEncodingException,
java.io.IOException
{ /* throws java.io.FileNotFoundException */
final java.io.FileInputStream fileInputStream
= new java.io.FileInputStream( sourceFile );
try
{ printFirstLineOf( fileInputStream ); }
finally
{ fileInputStream.close(); }
> }
Click to expand...

All those extra methods seem futile to me. I would do this:

public static void printFirstLineOf( final java.io.File sourceFile )
throws
java.io.FileNotFoundException,
java.io.UnsupportedEncodingException,
java.io.FileNotFoundException,
java.io.IOException
{
BufferedReader reader
= new BufferedReader(new java.io.FileReader( sourceFile ));
try
{ System.out.println(read.readLine()); }
finally
{ reader.close(); }
}

and I don't understand the fixation with final variables either really.
 
K

Knute Johnson

Lew said:
Unless you return from a failed attempt to allocate a non-null br.

BufferedReader br;
try
{
br = new BufferedReader( new FileReader( file ));
}
catch ( IOException exc )
{
String msg = "Open BufferedReader: "+ exc.getLocalizedMessage();
logger.error( msg );
logger.debug( "stack", exc );
return;
}
assert br != null;

try
{
...
}
catch ( IOException exc )
{
String msg = "Use BufferedReader: "+ exc.getLocalizedMessage();
logger.error( msg );
logger.debug( "stack", exc );
}
finally
{
try
{
br.close();
}
catch ( IOException exc )
{
String msg = "Close BufferedReader: "+ exc.getLocalizedMessage();
logger.error( msg );
logger.debug( "stack", exc );
}
}
Click to expand...

That's a lot of extra code to avoid an if :).
 
R

Roedy Green

and I don't understand the fixation with final variables either really.
Click to expand...
If I were redesigning Java I would make all variables final by
default.

It is mainly as a documentation aid to put final on. WHEN IT IS
MISSING, it warns you the value will be redefined lower down in the
code. I would guess a compiler could deduce the finality even when it
not explicitly.

It also prevents errors of accidentally defining something you think
is final, e.g. assigning to the wrong variable.
 
J

Jack

Stefan Ram a écrit :
Roedy Green said:
Otherwise you need two try blocks. I have a strong aversion to any
extra nesting.
Click to expand...

Thanks for all answers!

Here is my current tutorial code. I accept some uneccessary
close()-calls (e.g., closing the input stream reader that might
already have been closed by closing the buffered reader.)

I have avoided visible nesting of two try clauses by splitting
the code into several methods.

The hypothetical problem is to print the first line of a text
file given by its path:

public class Main
{
public static void main( final java.lang.String[] args )
throws
java.io.FileNotFoundException,
java.io.UnsupportedEncodingException,
java.io.IOException
{ final java.lang.String sourceFileName = "C:/example.txt";
printFirstLineOf( sourceFileName ); }

public static void printFirstLineOf( final java.lang.String sourceFileName )
throws
java.io.FileNotFoundException,
java.io.UnsupportedEncodingException,
java.io.IOException
{ final java.io.File sourceFile = new java.io.File( sourceFileName );
printFirstLineOf( sourceFile ); }

public static void printFirstLineOf( final java.io.File sourceFile )
throws
java.io.FileNotFoundException,
java.io.UnsupportedEncodingException,
java.io.IOException
{ /* throws java.io.FileNotFoundException */
final java.io.FileInputStream fileInputStream
= new java.io.FileInputStream( sourceFile );
try
{ printFirstLineOf( fileInputStream ); }
finally
{ fileInputStream.close(); }}

public static void printFirstLineOf( final java.io.FileInputStream fileInputStream )
throws
java.io.UnsupportedEncodingException,
java.io.IOException
{ /* throws java.io.UnsupportedEncodingException */
final java.io.InputStreamReader inputStreamReader
= new java.io.InputStreamReader( fileInputStream, "UTF-8" );
try
{ printFirstLineOf( inputStreamReader ); }
finally
{ inputStreamReader.close(); }}

public static void printFirstLineOf( final java.io.InputStreamReader inputStreamReader )
throws java.io.IOException
{ final java.io.BufferedReader bufferedReader
= new java.io.BufferedReader( inputStreamReader );
try
{ printFirstLineOf( bufferedReader ); }
finally
{ bufferedReader.close(); }}

public static void printFirstLineOf( final java.io.BufferedReader bufferedReader )
throws java.io.IOException
{ /* throws java.io.IOException */
final java.lang.String line = bufferedReader.readLine();
java.lang.System.out.println( line ); }}
Click to expand...

Why opening the BufferedReader before entering the try ?
 
H

Hendrik Maryns

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Roedy Green schreef:
| On Fri, 13 Jun 2008 04:07:18 GMT, EJP
| someone who said :
|
|> and I don't understand the fixation with final variables either really.
| If I were redesigning Java I would make all variables final by
| default.
|
| It is mainly as a documentation aid to put final on. WHEN IT IS
| MISSING, it warns you the value will be redefined lower down in the
| code. I would guess a compiler could deduce the finality even when it
| not explicitly.
|
| It also prevents errors of accidentally defining something you think
| is final, e.g. assigning to the wrong variable.

Eclipse now has Save Actions, which add ‘final’ whereever possible. I
like this very much. (Of course it can do much more, such as format
code, surround ifs with braces etc.)

H.
- --
Hendrik Maryns
http://tcl.sfs.uni-tuebingen.de/~hendrik/
==================
http://aouw.org
Ask smart questions, get good answers:
http://www.catb.org/~esr/faqs/smart-questions.html
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.4-svn0 (GNU/Linux)
Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org

iD8DBQFIUkzhe+7xMGD3itQRAgtqAJ0TArrVuRqwm/WlabToN3lc4hDV+QCfSc3C
u+CAxBido8p0uRZ7aQsjDh8=
=gATe
-----END PGP SIGNATURE-----
 
J

jolz

BufferedReader br = null;
try {
br = new BufferedReader(
new InputStreamReader(
new FileInputStream("file")));

// read data
} catch (IOException ioe) {
// deal with exception
} finally {
if (br != null)
try {
br.close();
} catch (IOException ioe) {
// deal with close exception
}
}
Click to expand...

Actually this code may leave FileInputStream not closed. If for example
OutOfMemoryError is thrown during BufferedReader construction it can't
be closed, so it won't close FileInputStream. Using classes like
ObjectInputStream that may throw IOException in constructor increases
odds of leaving streams open.
 
L

Lew

Actually this code may leave FileInputStream not closed. If for example
OutOfMemoryError is thrown during BufferedReader construction it can't
be closed, so it won't close FileInputStream. Using classes like
ObjectInputStream that may throw IOException in constructor increases
odds of leaving streams open.
Click to expand...

If it throws OOME, the program will crash, thus closing all IO
streams.
 
J

jolz

If it throws OOME, the program will crash, thus closing all IO
streams.
Click to expand...

It won't crash. Application will catch the error and presend appropriate
message. Afterwards it will continue with other tasks if current task
isnt't essential for the application (and most task aren't, especially
in applications with user interface).
 
K

Knute Johnson

Lew said:
Why are people so rabid, knee-jerk negative about six extra lines of code?

It's not just "to avoid an if". It's to provide fine-grained control of
the exception handling and logging.

Sheesh. People are so lazy.
Click to expand...

Next time instead of :) I'll use 'tongue-in-cheek'.
 
K

Knute Johnson

Lew said:
Ah.

One of my colleagues, a thoughtful and wise person who disagrees with a
whole lot of my notions, prefers the single try-catch with no
middle-of-the-method 'return', assessing my approach as marginally
acceptable but inferior to the approach cited by others in this thread.

While I do assert that my suggestion admits of finer-grained logging, I
do not assert that finer-grained logging is always desirable.

Strengths of a separate try-catch to open a resource:
- assertability of the resource's successful acquisition
- simpler finally block to release it
- single assignment to resource handle rather than repeated
- precise logging

Weaknesses:
- overhead of additional try block
- repetitiveness of additional try block per resource, with logging and
error messages each time
- multiple exit points per method
Click to expand...

It all comes down to what do you need.
 
J

jolz

Your problem is that you're doing the wrong thing
in the first place.
Click to expand...

The problems is that someone (probably sun) somewhere wrote that Errors
shouldn't be caught, which isn't true. Even sun sometimes is doing it
the right way - if Netbeans is out of memory it will properly catch the
error, show the message, an will be able to continue with other tasks.
Sometimes it's doing the wrong way - if I remember correctly
MediaTracker.waitForAll() will hang if it tries load too big image,
because Throwable is thrown in another thread - it should rethrow the
exception, so user waiting on waitForAll() knows that image was too big.
Every application that has File->Open must be prepared that user selects
something big and cannot crash. It must say "The file is too big -
application is suppose to work with smaller files and no developer will
ever bother to implement patrtial loading because you decided to open a
movie file in a text editor". Even without File->Open: if in application
server some application decides to allacate all memory it cannot crash
all other running applications. Throwable will be caught, logged, and
other application won't even notice it. Ignoring OutOfMemoryError is
like ignoring NULL returned by malloc and should never be done (and
fortunatelly many times isn't).
 
C

Christian

Stefan said:
When creating and using a file input stream,
what is the most recommendable boilerplate?

Or, what should be taught in a tutorial?

This first possibility?

{ final java.io.FileInputStream fileInputStream
= new java.io.FileInputStream( sourceFile );
/* use the fileInputStream */ }

Or this second possibility?

{ final java.io.FileInputStream fileInputStream
= new java.io.FileInputStream( sourceFile );
try
{ /* use the fileInputStream */ }
finally
{ fileInputStream.close(); }}

~~

The following code does not contain »finally« for
simplification, but is it good, unneccessary, or harmful to
write all these close()s in this order?

final java.io.FileInputStream fileInputStream
= new java.io.FileInputStream( sourceFile );

final java.io.InputStreamReader inputStreamReader
= new java.io.InputStreamReader( fileInputStream, "UTF-8" );

final java.io.BufferedReader bufferedReader
= new java.io.BufferedReader( inputStreamReader );

/* ... use bufferedReader here ... */

bufferedReader.close();
inputStreamReader.close();
fileInputStream.close();
Click to expand...

I ususally have some helper method somewhere.
Its pretty useful as you/I seldom will react to exceptions on close
So finally finally becomes a one-liner again.

public static void close(Closeable... closeable) {
for (Closeable c: closeable) {
try {
if (c != null) {
c.close();
}
} catch (IOException e) {
logger.debug(e,e);
}
}
}
 
J

Jim

This is not valid. How do you avoid a compilation error?


I get:

Compiling 1 source file to ~/projects/testit/build/classes
~/projects/testit/src/testit/Impatiex.java:29:
variable is might not have been initialized
if (is != null)
1 error
Click to expand...
True, I was just writing from memory........

Jim
 

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

How to slurp/get the content of a URI? 13
Closing decorated streams 6
Do I have to close all streams from Process Class after using theProcess object? 3
GIF file encoding to save and display in browser 4
Questions about buffered streams 28
I develop a Java program to format Java codes 14
Reading from a socket: first characters, then octets 8
MultiProperties 1

Members online

No members online now.
Total: 42 (members: 0, guests: 42)
Robots: 577

Forum statistics

Threads
473,769
Messages
2,569,578
Members
45,052
Latest member
LucyCarper

Latest Threads

Top