Meaningless Size Comparison
ID |
java.meaningless_size_comparison |
Severity |
high |
Remediation Complexity |
trivial |
Remediation Risk |
low |
Remediation Effort |
low |
Resource |
Reliability |
Language |
Java |
Tags |
reliability, suspicious-comparison |
Description
Reports a Collection/Map size or array length compared against a bound the value can never
cross, which makes the comparison a constant. size() and .length can never be negative, so
>= 0 and > -1 always hold, while < 0 and ⇐ -1 never do.
Rationale
A comparison that cannot change its answer either guards nothing, or — far more often — was meant to be an emptiness check written with the wrong bound.
void report(List<String> items, int[] slots) {
if (items.size() >= 0) { // FLAW — always true, the guard does nothing
System.out.println(items.get(0));
}
if (slots.length < 0) { // FLAW — always false, this branch is dead
System.out.println("no slots");
}
}The first branch always runs, including for an empty items, so the bug surfaces later as an
IndexOutOfBoundsException. The second branch never runs, so whatever it was meant to handle is
silently skipped.
Both operand orders are recognised, and only the bounds 0 and -1 are considered — every other
bound (> 0, == 0, >= 42, …) expresses a real condition and is left alone.
Remediation
Decide what the test was meant to say and write that bound: "has elements" is size() > 0; "is
empty" is size() == 0 (or the dedicated isEmpty()). If the comparison was defensive and the
value genuinely cannot be negative, delete it — it adds no protection and only distracts from the
checks that do.
void report(List<String> items, int[] slots) {
if (!items.isEmpty()) { // OK — genuine emptiness check
System.out.println(items.get(0));
}
if (slots.length > 0) { // OK — a real bound
System.out.println(slots[0]);
}
}