java
oopIt often happens, especially in Java, that a few places in the method are potential exception originators. Usually, we make a large method-size try block with a single catch at the bottom. We catch all the exceptions, usually even using grouping. This helps us minimize the noise, which is the exception catching. However, such large try blocks jeopardize maintainability: we are unable to provide proper error context inside catch blocks.
What do you think is wrong with this Java method (aside from using System.out instead of an injected dependency)?:
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.regex.Pattern;
void grep(Path file, Pattern regex) {
try {
for (String line : Files.readAllLines(file)) {
if (regex.matcher(line).matches()) {
System.out.println(line);
}
}
} catch (IOException ex) {
throw new IllegalStateException(ex);
}
}
I believe that its try/catch block is too big. The IOException may only be thrown by the readAllLines static method, but the block covers a few other method calls and statements. This code would be better:
void grep(Path file, Pattern regex) {
String[] lines;
try {
lines = Files.readAllLines(file);
} catch (IOException ex) {
throw new IllegalStateException(ex);
}
for (String line : lines) {
if (regex.matcher(line).matches()) {
System.out.println(line);
}
}
}
Now the try/catch block covers exactly the place where the exception may originate. Nothing else!
Why are smaller try-blocks better? Because they allow more focused error reporting with more detailed context. For example, the second snippet can be re-written as follows:
void grep(Path file, Pattern regex) {
String[] lines;
try {
lines = Files.readAllLines(file);
} catch (IOException ex) {
throw new IllegalStateException(
String.format(
"Failed to read all lines from %s",
file
),
ex
);
}
for (String line : lines) {
if (regex.matcher(line).matches()) {
System.out.println(line);
}
}
}
Can we do the same with the first snippet? We could, but the error message would be inaccurate, because the block covers too much.
