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.
Source: javacodegeeks.com
0 comments:
Post a Comment