Empty for body, else branch or do body
ID |
apex.correctness.empty_statement_block |
Severity |
low |
Remediation Complexity |
trivial |
Remediation Risk |
low |
Remediation Effort |
low |
Resource |
Dead Code |
Language |
Apex |
Description
Reports an empty for body (both the for-each and the counted form), an empty else branch, and an empty do body.
Rationale
Each of these costs something and returns nothing. An empty for still walks the whole collection and still counts every iteration against the CPU limit, so a loop over a batch scope burns time to produce no effect. An empty do body is worse: the loop re-tests a condition nothing inside it can change, so it either exits immediately or spins until the transaction is killed on the CPU limit. An empty else states that a case was considered and then leaves it unhandled, which is indistinguishable from the branch whose body was deleted during a merge - and that is usually what it is.
The following code illustrates the pattern detected by this rule:
public class BatchSummariser {
public static Integer countAccounts(List<Account> accounts) {
Integer total = 0;
// FLAGGED: Empty for body, else branch or do body
for (Account a : accounts) {
}
return total;
}
public static void warmUp(Integer rounds) {
// FLAGGED: Empty for body, else branch or do body
for (Integer i = 0; i < rounds; i++) {
}
}
public static String label(Account a) {
String text;
// FLAGGED: Empty for body, else branch or do body
if (a.Name != null) {
text = a.Name;
} else {
}
return text;
}
public static void drain(Integer pending) {
// FLAGGED: Empty for body, else branch or do body
do {
} while (pending > 0);
}
public static Integer countAccountsFixed(List<Account> accounts) {
Integer total = 0;
for (Account a : accounts) {
total++;
}
return total;
}
public static String labelFixed(Account a) {
String text;
if (a.Name != null) {
text = a.Name;
} else {
text = 'unnamed';
}
return text;
}
public static void noop(Account a) {
if (a.Name == null) {
}
}
public static void spin(Boolean pending) {
while (pending) {
}
}
public static void neither(Account a) {
if (a.Name == null) {
} else {
}
}
}Remediation
Restore the work the block was meant to do, or delete the construct entirely: drop the loop, and drop the else if the if alone says what is meant. Where a branch is deliberately a no-op, say so with a comment naming the case, so the next reader is not left deciding whether something was lost.