user2997204 user2997204 - 4 months ago 13
Java Question

bad try catch block design?

Whenever I use try-catch blocks in my code, my code always follows a pattern.
First a try-catch block to open a resource followed by a null check and finally a try-catch block to read the resources.
Is this a messy pattern? If so, what should the good design look like?

Here is an example of what I mean

public static void main(String[] args) {
Process process = null;
try {
process = Runtime.getRuntime().exec("C:\\program.exe");
} catch (IOException e) {
e.printStackTrace();
}
if (process == null) {
return;
}
try (BufferedReader reader = new BufferedReader(new InputStreamReader(process.getInputStream()))) {
String line;
while ((line = reader.readLine()) != null) {
System.out.println(line);
}
} catch (IOException e) {
e.printStackTrace();
}
}

Answer

Your problem is that you are suppressing the exceptions in your code, which is why you need the null check. Particularly since this is in a main() method you could just let the exceptions propagate:

public static void main(String[] args) throws IOException {
  Process process = Runtime.getRuntime().exec("C:\\program.exe");
  try (BufferedReader reader =
      new BufferedReader(new InputStreamReader(process.getInputStream()))) {
    String line;
    while ((line = reader.readLine()) != null) {
      System.out.println(line);
    }
  }
}

This will still print a stack trace when there's an issue, but you know that process can never be null. In general you should avoid over-eagerly catching exceptions - only catch them if there's something you can do about it. If not, pass them on to the caller (possibly wrapping them in a new exception type).

If you just don't care about the exceptions and don't expect them in practice you can make them RuntimeExceptions instead. This still lets you avoid needless null-checks but also doesn't require you to annotate your methods with throws:

public static void main(String[] args) {
  try {
    Process process = Runtime.getRuntime().exec("C:\\program.exe");
    try (BufferedReader reader =
        new BufferedReader(new InputStreamReader(process.getInputStream()))) {
      String line;
      while ((line = reader.readLine()) != null) {
        System.out.println(line);
      }
    }
  } catch (IOException e) {
    throw new IllegalStateException(
        "Unexpected exception while executing a subprocess", e);
  }
}
Comments