Else If Needs Else

ID

csharp.else_if_needs_else

Severity

high

Remediation Complexity

trivial

Remediation Risk

low

Remediation Effort

low

Resource

Reliability

Language

CSharp

Tags

conditional, control-flow, reliability

Description

Reports an if / else if chain that does not end with a plain else. Only chains containing at least one else if are reported, and each chain is reported once, at its outermost if. A lone if, and a plain if / else, are not reported.

Rationale

Repeating a condition turns the chain into a decision table, and the reader takes the missing final else to mean the listed conditions cover every input. When they do not, control simply leaves the chain with whatever state it already had — the branch nobody wrote a test for. A final else costs one line and either handles the remaining case or documents, by throwing or logging, the author’s belief that there is none.

public decimal Fee(string tier)
{
    decimal fee = 0m;

    if (tier == "gold")               // FLAW — an unknown tier silently pays nothing
        fee = 1m;
    else if (tier == "silver")
        fee = 2m;

    return fee;
}

public decimal FeeChecked(string tier)
{
    if (tier == "gold")               // OK — the remaining case is stated
        return 1m;
    else if (tier == "silver")
        return 2m;
    else
        throw new ArgumentOutOfRangeException(nameof(tier));
}

Remediation

Close the chain with an else. Handle the remaining case if there is one; otherwise throw ArgumentOutOfRangeException, log the unexpected value, or return a well-defined default, so that an input outside the enumerated conditions is impossible to miss. When the chain tests one value against several constants, a switch expression is often the clearer form and lets the compiler check exhaustiveness.

References