14

Basically, I want to open a file, read some bytes, and then close the file. This is what I came up with:

try
{
    InputStream inputStream = new BufferedInputStream(new FileInputStream(file));
    try
    {
        // ...
        inputStream.read(buffer);
        // ...
    }
    catch (IOException e)
    {
        // TODO Auto-generated catch block
        e.printStackTrace();
    }
    finally
    {
        try
        {
            inputStream.close();
        }
        catch (IOException e)
        {
            // TODO Auto-generated catch block
            e.printStackTrace();
        }
    }
}
catch (FileNotFoundException e)
{
    // TODO Auto-generated catch block
    e.printStackTrace();
}

Maybe I'm spoiled by RAII, but there must be a better way to do this in Java, right?

fredoverflow
  • 256,549
  • 94
  • 388
  • 662
  • What do you want to do ? Avoid the handling of all those exceptions ? – user703016 Jun 06 '11 at 09:49
  • And how you would define better ? – Damian Leszczyński - Vash Jun 06 '11 at 09:49
  • Might be better off at http://codereview.stackexchange.com – Kevin Jun 06 '11 at 09:52
  • 3
    @Vash: I can't believe I need that much boilerplate code and nested try/catch blocks to perform such a simple task. – fredoverflow Jun 06 '11 at 09:52
  • 3
    @Heandel, @Vash: I count 21'ish lines of code in that sample, not counting the comments. And all he wanted to do was "open file, read data, close file. If an error occurs, still close file, but also print exception info". That should be 3, maybe 4 lines of code. If you *need* to ask how the code could have been better, you seriously need to start learning other languages than Java. – jalf Jun 06 '11 at 10:17
  • if you don't handle the exception a single try/catch is enough. – bestsss Jun 06 '11 at 10:29
  • @jalf, You can do it in one line, but the formating is not the case. Also knowing other languages will not give the possibility to read in some person mind. Never then less this is possibly the correct way to handling this problem, in this specific case we can use the exception hierarchy (vitaut already presented). But generally you need so many try catch code for this kind of operation. The difference between Java and most of other languages is that the compiler do not force you to handle exception that might occur. – Damian Leszczyński - Vash Jun 06 '11 at 11:21
  • 2
    @Vash: no, in some languages you do *not* need this many try/catch blocks for this kind of operation, and it has *nothing* to do with the compiler forcing you to handle exceptions. As I said before, you need to start learning some other languages. Open your eyes. Being forced to write ugly code because you're working in a crippled language is bad enough, but *not even realizing* that a better solution could exist is pretty much inexcusable for a programmer. – jalf Jun 06 '11 at 11:26
  • You have right that you are forced, but in 7th version of Java will be something like using from C#. But for this case in Java there is not better shot than code presented by vitaut. I've open eyes that is why knowing that in other languages you could do this using less lines does not change anything in Java source and generally there not exists solution that provide properly resource management with less code. So this is not the case that I don't know other languages, because you are wrong with this these, but that in Java this is done this way not another. – Damian Leszczyński - Vash Jun 06 '11 at 11:38
  • 1
    Guys, it is possible to write ugly code in any language, not only Java. The reason why this example is clunky is because it is written in such a way. One can even get rid of all the `catch` clauses by propagating the exception. Of course you can't get rid of the `finally` clause if you want to properly close the file. However, from my point of view this is a minor inconvenience and it will be addressed by Java 7 as I mentioned in the answer. The main reason automatic resource management is not as useful in Java as it is in C++ is due to GC. And I mostly use C++ BTW so I am not a Java advocate. – vitaut Jun 06 '11 at 12:03
  • 1
    @jalf: 21+ lines? I can write the code that does the same in 1000 lines of code but it doesn't mean that I have to do so =D. The language doesn't require you to catch after every line of code. – vitaut Jun 06 '11 at 13:22
  • 2
    @Vash: once again, no, other languages *do* have solutions which provide proper robust resource management without having to write a single try/catch. C++ can do it. Wanting to print out exception information might require *one* try/catch block, but only for the printing. Opening, reading from, and closing the file can be done *with perfect error handling*, without writing a single try-catch, and certainly without a `finally`. – jalf Jun 06 '11 at 13:45
  • 1
    @vitaut: I'd be more impressed if you could write code that does the same in less than 10 lines, without sacrificing correctness. Can you? I can in C++. – jalf Jun 06 '11 at 13:46
  • @jalf: Oh I thought it is the IOCCC. I can even write it in one line if the code brevity has become a requirement `buffer = readAllBytes();` Now your turn =D – vitaut Jun 06 '11 at 14:33
  • 1
    @vitaut: fine, make that "without sacrificing correctness *or readability*". I have to wonder if you're just being stupid to annoy me, or if you are literally so blinded by Java that you can't wrap your head around the idea that "the code could have been simpler to express in another language" – jalf Jun 06 '11 at 15:01
  • @jalf: Doesn't matter if you didn't get what I meant. Just want to say that you've made a good point that one should learn several languages. Otherwise too much flame. – vitaut Jun 06 '11 at 15:30
  • You maybe interested in this article about Java checked vs unchecked exception: http://www.mindview.net/Etc/Discussions/CheckedExceptions – Alvin Jun 06 '11 at 19:06
  • The new try-with-resources feature of Java 7 solves this problem, by recording but "suppressing" exceptions from `close()`: http://stackoverflow.com/questions/7849416/what-is-a-suppressed-exception – Raedwald Oct 21 '11 at 12:57

9 Answers9

12

If you have the same exception handling code for IOException and FileNotFoundException then you can rewrite your example in a more compact way with only one catch clause:

try {
    InputStream input = new BufferedInputStream(new FileInputStream(file));
    try {
        // ...
        input.read(buffer);
        // ...
    }
    finally {
        input.close();
    }
}
catch (IOException e) {
    e.printStackTrace();
}

You can even get rid of the outer try-catch if you can propagate the exception which probably makes more sense then manually printing the stack trace. If you don't catch some exception in your program you'll get stack trace printed for you automatically.

Also the need to manually close the stream will be addressed in Java 7 with automatic resource management.

With automatic resource management and exception propagation the code reduces to the following:

try (InputStream input = new BufferedInputStream(new FileInputStream(file))) {
    // ...
    input.read(buffer);
    // ...
}
vitaut
  • 49,672
  • 25
  • 199
  • 336
  • 4
    So in its 7th version, about 15 years after its first release, Java will finally (no pun intended) provide one of the major features of the language it was created to improve upon. I am not impressed. – sbi Jun 06 '11 at 11:18
  • @sbi: and it's still failing to provide the same feature, instead settling for merely cutting away some of the boilerplate code. Java7's resource management is more like C#'s Using than C++'s RAII. Far better than what they have now, but still not as good as, like you said, the major feature of the language it was intended to improve upon. – jalf Jun 06 '11 at 11:29
  • 1
    Oh, no! My answer is being hijacked by C++ advocates. =) – vitaut Jun 06 '11 at 11:37
  • 1
    Yeah. It's funny. The question could be closed for _questions of this type [...] usually lead to confrontation and argument_. Of course it won't because it highlights a valid point. Just saying... – musiKk Jun 06 '11 at 11:45
  • I doubt if this is correct. stream is opened in constructor and used by read. What is stream is opened and then exception was thrown in constructor ? would it not leak resource ? – JavaDeveloper Mar 12 '14 at 03:43
  • 1
    @JavaDeveloper If FileInputStream ctor throws an exception it should free resources and BufferedInputStream doesn't throw anything (see http://docs.oracle.com/javase/7/docs/api/java/io/BufferedInputStream.html#BufferedInputStream(java.io.InputStream)). So it shouldn't leak resources. – vitaut Mar 12 '14 at 03:49
2

Usually these methods are wrapped up in libraries. Unless you want to write at this level, it is best to create your own helper methods or use existing ones like FileUtils.

String fileAsString = Fileutils.readFileToString(filename);
// OR
for(String line: FileUtils.readLines(filename)) {
    // do something with each line.
}
Peter Lawrey
  • 525,659
  • 79
  • 751
  • 1,130
1

I don't know if it is the right way, but you can have all your code in the same try block, and then have the different catch blocks right after each other.

try {
  ...
}
catch (SomeException e) {
  ...
}
catch (OtherException e) {
  ...
}
Rasmus Øvlesen
  • 510
  • 3
  • 8
1

If you want to do this in plain Java, the code you have posted looks OK.

You could use 3rd party libraries such as Commons IO, in which you would need to write much less code. e.g.

Check out Commons IO at:

http://commons.apache.org/io/description.html

Dave
  • 1,303
  • 7
  • 19
1

Sometimes you can reduce the code to the following:

public void foo(String name) throws IOException {
    InputStream in = null;
    try {
        in = new FileInputStream(name);
        in.read();
        // whatever
    } finally {
        if(in != null) {
            in.close();
        }
    }
}

Of course this means the caller of foo has to handle the IOException but this should be the case most of the time anyway. In the end you don't really reduce all that much complexity but the code becomes much more readable due to less nested exception handlers.

musiKk
  • 14,751
  • 4
  • 55
  • 82
1

My take on this without using utilities would be:

InputStream inputStream = null;
try {
    inputStream = new BufferedInputStream(new FileInputStream(file));
    // ...
    inputStream.read(buffer);
    // ...
} catch (IOException e) {
    e.printStackTrace();
    throw e; // Rethrow if you cannot handle the exception
} finally {
    if (inputStream != null) {
        inputStream.close();
    }
}

Not a one-liner, but not pretty bad. Using, say, Apache Commons IO it would be:

//...
buffer = FileUtils.readFileToByteArray(file);
//...

The thing to rembember is that standard Java lacks many of these little utilities and easy to use interfaces that everyone needs, so you have to rely on some support libraries like Apache Commons, Google Guava, ... in your projects, (or implement your own utility classes).

gpeche
  • 21,974
  • 5
  • 38
  • 51
0

Use org.apache.commons.io.FileUtils.readFileToByteArray(File) or something similar from that package. It still throws IOException but it deals with cleaning up for you.

artbristol
  • 32,010
  • 5
  • 70
  • 103
0

Try the following:

try
{
    InputStream inputStream = new BufferedInputStream(new FileInputStream(file));
    byte[] buffer = new byte[1024];
    try
    {
        // ...
        int bytesRead = 0;
        while ((bytesRead = inputStream.read(buffer)) != -1) {                
           //Process the chunk of bytes read
        }
    }
    catch (IOException e)
    {
        // TODO Auto-generated catch block
        e.printStackTrace();
    }
    finally
    {           
        inputStream.close();
    }
}
catch (FileNotFoundException e)
{
    // TODO Auto-generated catch block
    e.printStackTrace();
}
Hasan Fahim
  • 3,875
  • 1
  • 30
  • 51
0

Google guava has tried to address this problem by introducing Closeables.

Otherwise you have to wait until AutoCloseable from JDK 7 comes out as it addresses some of the cases when IOException is thrown.

mindas
  • 26,463
  • 15
  • 97
  • 154