4
\$\begingroup\$

Which of the following is cleaner / more adopted standard of handling exceptions while coding a stream ? I would also appreciate a reason why one of the following trumps over other ? In my opinion I find option 1 better but nested try where first try does nothing more than a wrapper is making me not satisfied with approach.

NavigableSet<String> dictionary = new TreeSet<String>();
BufferedReader br = null;
try {
    try {
        br = new BufferedReader(new FileReader("/Users/ameya.patil/Desktop/text.txt"));
        String line;
        while ((line = br.readLine()) != null) {
            dictionary.add(line.split(":")[0]);
        }

    } finally {
        br.close();
    }
} catch (Exception e) {
    throw new RuntimeException("Error while reading dictionary");
}

VS

NavigableSet<String> dictionary = new TreeSet<String>();
BufferedReader br = null;

    try {
        br = new BufferedReader(new FileReader("/Users/ameya.patil/Desktop/text.txt"));
        String line;
        while ((line = br.readLine()) != null) {
            dictionary.add(line.split(":")[0]);
        }

    }
} catch (Exception e) {
    throw new RuntimeException("Error while reading dictionary");
} finally {
     try{
        if(br != null) br.close();
        } catch (Exception ex){

        }
 }
\$\endgroup\$

3 Answers 3

4
\$\begingroup\$

For the second example you posted, I do not think finally is used in a correct way. using try in finally block is not a good idea you can check the documentation

The finally block always executes when the try block exits. This ensures that the finally block is executed even if an unexpected exception occurs. But finally is useful for more than just exception handling — it allows the programmer to avoid having cleanup code accidentally bypassed by a return, continue, or break. Putting cleanup code in a finally block is always a good practice, even when no exceptions are anticipated.

I have modified the first version here...

NavigableSet<String> dictionary = new TreeSet<String>();                                                                                                                                                                                                                          
BufferedReader br = null;                                                                                                                                                                                                                                                         
try {                                                                                                                                                                                                                                                                             
  br = new BufferedReader(new FileReader("/Users/ameya.patil/Desktop/text.txt"));                                                                                                                                                                                                 
  String line = null;                                                                                                                                                                                                                                                             
  while ((line = br.readLine()) != null) {                                                                                                                                                                                                                                        
    dictionary.add(line.split(":")[0]);                                                                                                                                                                                                                                           
  }                                                                                                                                                                                                                                                                               
} catch(FileNotFoundException e) {                                                                                                                                                                                                                                                
  System.err.println("Caught FileNotFoundException: " + e.getMessage());                                                                                                                                                                                                          
  throw new RuntimeException(e);                                                                                                                                                                                                                                                  
} catch(IOException e) {                                                                                                                                                                                                                                                          
  System.err.println("Caught IOException: " + e.getMessage());                                                                                                                                                                                                                    
}finally {                                                                                                                                                                                                                                                                        
  if (null != br) { br.close(); }                                                                                                                                                                                                                                                 
}
\$\endgroup\$
3
  • 1
    \$\begingroup\$ It was a while since I worked with Java, but I remember you had to enclose br.close() in a try/catch. In that case @JavaDevelopers second example was fine as it was. \$\endgroup\$
    – Max
    Commented Oct 29, 2013 at 8:30
  • 1
    \$\begingroup\$ well, @JavaDeveloper's second version of code is not a bad practice, but for readability I would avoid that. Here is a discussion on link this \$\endgroup\$
    – Kinjal
    Commented Oct 29, 2013 at 8:37
  • 1
    \$\begingroup\$ I appreciate the feedback and the posted link, which is so useful \$\endgroup\$ Commented Oct 29, 2013 at 19:11
5
\$\begingroup\$

First off, in Java 7 all of this is obsolete if you use 'try with resources'

NavigableSet<String> dictionary = new TreeSet<>();
try (BufferedReader br = new BufferedReader(new FileReader("/Users/ameya.patil/Desktop/text.txt"))) {
    String line;
    while ((line = br.readLine()) != null) {
        dictionary.add(line.split(":")[0]);
    }
} catch (Exception e) {
    throw new RuntimeException("Error while reading dictionary");
}

If you have to work on an earlier JDK consider these points :

Both snippets you supply are not exactly equivalent. i.e. if closing the file fails, the first snippet will throw a RuntimeException wrapping the exception of the close(). The second snippet will throw a RuntimeException wrapping the original exception.

Again in SE 7 using the try with resources you'll be able to see both exceptions : the original exception is thrown and it has the exception upon close() attached as a suppressed exception.

So I think style is irrelevant, you want the original exception (which may even not be related to I/O). If you can use SE 7, use that, otherwise go for snippet 2 (add logging for the exception on close())

\$\endgroup\$
2
\$\begingroup\$

My usual answer in cases like this is to isolate the construction of the object from the consumption of it. If you split up these two methods, then you don't have to worry about branching in the finally clause -- if you have reached the method responsible for closing the object, then you finally close it.

BufferedReader br = createReader("/Users/ameya.patil/Desktop/text.txt");
NavigableSet<String> dictionary = createDictionary(br);

...
BufferedReader createReader(String fileName) {
    try {
        return new BufferedReader(new FileReader(fileName));
    } catch (IOException e) {
        throw new RuntimeException("Unable to create buffer: " + fileName, e);
    }
}

NavigableSet<String> createDictionary(BufferedReader br) {
    try {
        NavigableSet<String> dictionary = new TreeSet<String>(); 
        while((line = br.readLine()) != null) {
            dictionary.add(line.split(":")[0]);
        }
        return dictionary;
    } finally {
        // If we are in this method at all, we KNOW we need to
        // close() the reader when we are done with it.
        br.close();
    }

You probably wanted to refactor out createDictionary() anyway, so that you can test it properly.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.