Meaningless Size Comparison

ID

csharp.meaningless_size_comparison

Severity

high

Remediation Complexity

trivial

Remediation Risk

low

Remediation Effort

low

Resource

Reliability

Language

CSharp

Tags

collection, reliability, suspicious-comparison

Description

Reports a collection size or array length compared against a bound the size can never cross, which makes the comparison a constant. Count, Length, LongLength and the Count() sequence extension all report how many elements there are and none of them can be negative, so >= 0 and > -1 always hold, while < 0 and ⇐ -1 never do.

Rationale

A comparison that cannot change its answer is either a guard that guards nothing or — far more often — an emptiness check that was written with the wrong bound. if (items.Count >= 0) reads like "if there is anything in the list" and behaves like "always", so the code inside runs for the empty list too and the bug surfaces later as an index out of range or an empty report. The mirrored form if (0 > items.Count) has the opposite failure: the branch is dead, and whatever it was supposed to handle is silently never handled.

Both operand orders are recognised. Only the bounds 0 and -1 are considered — every positive bound expresses a real condition. Matching is by member name on a receiver, so a plain identifier named Count belonging to the enclosing type is not treated as a size.

public class Cart
{
    private readonly List<string> items = new List<string>();
    private readonly int[] slots = new int[8];

    public void Report()
    {
        if (items.Count >= 0)          // FLAW — always true, the guard does nothing
        {
            Console.WriteLine(items[0]);
        }

        if (slots.Length < 0)          // FLAW — always false, this branch is dead
        {
            Console.WriteLine("no slots");
        }

        if (slots.LongLength >= 0)     // FLAW — the 64-bit length is never negative either
        {
            Console.WriteLine(slots[0]);
        }

        if (items.Count > 0)           // OK — genuine emptiness check
        {
            Console.WriteLine(items[0]);
        }

        if (slots.Length >= 2)         // OK — a real bound
        {
            Console.WriteLine(slots[1]);
        }
    }
}

Remediation

Decide what the test was meant to say and write that bound. "Has elements" is Count > 0 (or Count != 0); "is empty" is Count == 0. For a sequence, prefer Any() over Count() > 0, as it stops at the first element. If the comparison was defensive and the value genuinely cannot be negative, delete it — it adds no protection and hides the reader’s eye from the checks that do.

References