Try/Catch Block In Loop
ID |
java.loop_try_in_loop |
Severity |
high |
Remediation Complexity |
trivial |
Remediation Risk |
low |
Remediation Effort |
low |
Resource |
Efficiency |
Language |
Java |
Tags |
efficiency |
Rationale
Setting up a try/catch frame on every loop iteration adds runtime overhead. When the catch only rethrows, the first exception ends the loop anyway, so the frame can be hoisted around the loop and the per-iteration setup disappears without changing what the program does.
// Bad -- the catch rethrows, so the loop ends on the first failure regardless
for (int i = 0; i < 10; i++) {
try {
riskyOperation(i);
} catch (IllegalStateException e) {
throw new IllegalArgumentException(e);
}
}Most try blocks in a loop are not that shape. They are per-iteration error isolation: the catch logs, ignores, records the failure or retries, and the loop keeps going. Hoisting one of those changes the program — the first failure would abort every remaining iteration — so none of the following is reported:
// OK -- the loop keeps going after a failure
for (String source : sources) {
try {
process(source);
} catch (Exception e) {
LOG.warn("skipping {}", source, e);
}
}
// OK -- the resource must be acquired and closed per iteration
for (Path directory : directories) {
try (Stream<Path> walk = Files.walk(directory)) {
walk.count();
} catch (IOException e) {
throw e;
}
}
// OK -- the try belongs to the lambda's body, not to the iteration
for (Path directory : directories) {
files.forEach(file -> {
try {
delete(file);
} catch (IOException e) {
throw new UncheckedIOException(e);
}
});
}A try with no catch at all (try/finally) is out of scope for the same reason: its finally is meant to run per iteration.
Remediation
Move the try/catch block outside the loop.
// Good -- one frame for the whole loop
try {
for (int i = 0; i < 10; i++) {
riskyOperation(i);
}
} catch (IllegalStateException e) {
throw new IllegalArgumentException(e);
}