tags:

views:

125

answers:

9

I'm learning java and one thing I've found that I don't like, is generally when I have code like this:

import java.util.*;
import java.io.*;

public class GraphProblem
{
    public static void main(String[] args)
    {
        if (args.length < 2)
        {
            System.out.println("Error: Please specify a graph file!");
            return;
        }


        FileReader in = new FileReader(args[1]);
        Scanner input = new Scanner(in);

        int size = input.nextInt();
        WeightedGraph graph = new WeightedGraph(size);

        for(int i = 0; i < size; i++)
        {
            graph.setLabel(i,Character.toString((char)('A' + i)));
        }

        for(int i = 0; i < size; i++)
        {
            for(int j = 0; j < size; j++)
            {
                graph.addEdge(i, j, input.nextInt());
            }
        }

        // .. lots more code

    }
}

I have an uncaught exception around my FileReader.

So, I have to wrap it in a try-catch to catch that specific exception. My question is does that try { } have to encompass everything after that in my method that wants to use either my FileReader (in) or my Scanner (input)?

If I don't wrap the whole remainder of the program in that try statement, then anything outside of it can't access the in/input because it may of not been initialized or has been initialized outside of its scope. So I can't isolate the try-catch to just say the portion that intializes the FileReader and close the try statement immediately after that.

So, is it the "best practice" to have the try statement wrapping all portions of the code that are going to access variables initialized in it?

Thanks!

+9  A: 

If you are comfortable with not wrapping the code after the FileReader constructor, you can declare the FileReader outside of the try/catch block, like this:

FileReader fr = null;
try
{
    fr = new FileReader(args[1]);
}
catch (IOException e)
{
    // handle
}
// code that uses fr

This is a reasonable design, and I use it often. Make sure you properly handle, in the following code, the possibility that fr is null (i.e. the constructor threw an exception).

Michael Petrotta
The variable `fr` must be initialized - this won't compile unless the exception handling code is guaranteed to return.
Kevin Brock
@Kevin - edited; thanks.
Michael Petrotta
Shouldn't you also have a finally to see if the fr is not null and call close on it?
CoolBeans
@CoolBeans: under what circumstances would an exception be thrown *and* `fr` be not null? The OP will need to manage the FileReader lifetime properly, of course, but that's outside the scope of this example.
Michael Petrotta
A: 

Yes and no. The Try initiates it's own local scope so when you do this:

try
{
   FileReader reader = new FileReader(args[1]);

   // Stuff
}
catch(Exception e) { }

The reader variable is only visible within the constraints of the try {} block. This is preferred behaviour. If you want to use it outside (in the case of file i/o it's not reccomended) you need to declare it outside of the try/catch. A proper example would be a flag:

boolean isValidFile = false;
try
{
   FileReader reader = new FileReader(args[1]);

   // Do Stuff

   isValidFile = true;
}
catch(Exception e)
{
   // Handle Errors
}

System.out.print("Valid File: ");
System.out.println(isValidFile);
Aren
A: 

You should definitely follow the pattern above. If I remember right from my SCJP studying, the compiler doesn't try to optimize anything that is in a try block, so you should try-catch at the lowest level possible.

BrennaSoft
Is this a comment or an answer? And don't use "above". Answers have no canonical ordering.
polygenelubricants
+2  A: 

No. You can declare it like this:

FileReader in = null;
Scanner input = null;

try {
   in = new FileReader(args[1]);
   input = new Scanner(in);
} catch(IOException ioe) {
   //
}
cs80
+2  A: 

This is not an issue with try/catch blocks as such. The issue is variable scoping and that you must have a try/catch block because of the checked exception and thus establish a new scope.

You also have another option - declare the checked exception(s) as throws from your method.

public static void main(String[] args) throws IOException {
    // ...code here...
}

This is perfectly legal for a main method.

If you do want to handle the exception, as you ought to in a larger program. You can define a specific try/catch around the problem block of code and use the variable outside of the scope by declare the variable outside of that scope as many have answered:

FileReader fr = null; // must be initialized here if the exception handling code 
                      // does not exit the method
try {
    fr = new FileReader(fileName);
} catch (IOException ex) {
    // log, print, and/or return
    // if you return or exit here then subsequent code can assume that fr is valid
}

You can also put the move the code to another method that deals with the exception:

private static FileReader openReader(String fileName) {
    try {
        return new FileReader(fileName);
    } catch (IOException ex) {
        // log/print exception
        return null; // caller must then expect a null
        // or
        throw new RuntimeException(...); // throw a RuntimeException of some kind (may not be good practice either)
    }
}

You could also move the file processing code to another method. This may be better and allow you to more correctly follow the open/close in finally idiom:

FileReader fr = null;
try {
    fr = new FileReader(fileName);
    Scanner input = new Scanner(fr);

    processInput(input);
} catch (IOException ex) {
    // log/print exception
} finally {
    if (fr != null) {
        try {
            fr.close();
        } catch (IOException ex) {
            // empty
        }
    }
}

private static void processInput(Scanner in) throws IOException {
    // ...processing here
}

For the close part, you could use a third party library (Apache File Utils) or write a simple method to provide a static safe close method that doesn't throw exceptions.

You really should look at breaking up large methods into smaller units and this will often provide you with a much clearer way of handling the exceptions as well.

Kevin Brock
One drawback of this method is that the Java default `printStackTrace()` doesn't print nested exceptions, so you just see the last one (which is especially irritation about `SQLException`s when the first exception says "Connection failed. See the next exception for details". Use commons-lang's `ExceptionUtils` or your own method to dump the whole tree.
Aaron Digulla
@Aaron: Which one of them? I present several and don't assume anything in what is done within the exception handler. I also state "log" as an option; logging APIs generally handle nested exceptions. I didn't reference printStackTrace either. Is this comment on the right answer?
Kevin Brock
You're implicitly using `printStackTrace()` if you write `void main(String[] args) throws IOException`.
Aaron Digulla
A: 

In addition to Michale Petrotta's answer another thing you should keep in mind is its always a good idea to have the try-catch block enclose only the statements that actually throw the exception.

What's the point in enclosing a statement in a try-catch block if its not gonna throw the exception?

And this would help a lot while debuging/maintaining your code.

Mihir Mathuria
A: 

If you are just learning, you can have your method throw Exception. It isn't a great way to handle exceptions however, if you are just learning, you probably want to focus more on the logic of what your code is doing. Once you are comfortable with that, then you can look into handling exceptions properly.

So your code should look like the following and you won't have to worry about try/catch statements:

public class GraphProblem {
    public static void main(String[] args) throws Exception {
        //your code
    }
}

Again, this is not a best practice but it allows you to focus on your business logic rather than error handling. Once you get comfortable with the business logic, then you can dig into exception handling which is a topic onto itself.

Chris J
A: 

One thing that would help in your case would be to segregate the file-reading code from the problem-solving code. If you read the contents of the file up front and store them into an in-memory data structure then you won't be interleaving your exception-prone IO with other code.

Here's an article on checked exceptions that might be helpful.

Nathan Hughes
A: 

Local variables declared within a block are scoped to that block.

So, is it the "best practice" to have the try statement wrapping all portions of the code that are going to access variables initialized in it?

In general, you should try to minimize the scope of your variables to make them as narrow as possible. This is covered in Effective Java and Code Complete, among others.

This sort of code is a recipe for NullPointerExceptions:

FileReader in = null;
try {
  in = new FileReader(filename);
} catch(IOException e) {
  //handle error, but now what?
}
// Code that needs "in", but what if it is null
// because of a FileNotFoundException?
// This code is junk.

Virtually all local variable declarations should include an assignment.

FileReader in = new FileReader(filename);

If you follow this rule of thumb, you'll end up with better code.


// .. lots more code

If you have giant try/catch statements, it sounds like your methods are too big. This is a code organization issue rather than something to do with try/catch specifically.

  public static void main(String[] args) {
    if (args.length < 1) {
      System.out.println("Error: Please specify a graph file!");
      return;
    }
    try {
      processGraphFile(args[0]);
    } catch (IOException e) {
      // Deal with error
    }
  }

  private static void processGraphFile(String filename) throws IOException {
    FileReader in = new FileReader(filename);
    try {
      Scanner scanner = new Scanner(in);
      processGraph(scanner);
      if (scanner.ioException() != null) {
        throw scanner.ioException();
      }
    } finally {
      in.close();
    }
  }

  //other methods

As an aside:

  • Note that Java arrays start at index 0, not 1
  • The FileReader is convenient while learning, but should generally be avoided due to encoding issues; this is unlikely to be a problem until the code leaves your machine
McDowell