Else clause follows an If branch that always leaves the block
ID |
vbnet.maintainability.else_after_return |
Severity |
low |
Remediation Complexity |
trivial |
Remediation Risk |
low |
Remediation Effort |
low |
Resource |
Control Flow |
Language |
VB.NET |
Description
Reports an If statement whose Then branch consists of a single statement that always leaves the block - Return, Throw, Exit Sub, Exit Function, Exit Property, Continue For, Continue While or Continue Do - and which is nevertheless followed by an Else or ElseIf clause. Because control never reaches the end of the Then branch, the Else adds nothing and its body can be dedented to follow the If. Only an unconditional terminator counts: a Then branch that ends by falling through, or one whose Return sits inside a further nested condition, genuinely needs its Else and is not reported.
Rationale
Where the Then branch cannot fall through, the Else states a mutual exclusion the control flow has already guaranteed, and the reader pays for that twice. The Else body is indented under a condition, so it reads as a special case that only applies when the test fails, when in fact it is the normal path of the procedure - the shape hides the main line of the code inside a branch. And because the condition now appears to matter in both directions, a reader checking whether the negation is right has to verify a redundant claim. The cost compounds: each such pair adds a nesting level, so a procedure with four sequential validations ends up four levels deep, with the real work at the bottom, when the same logic written as guard clauses stays flat. That depth is also what makes the branch easy to get wrong during edits - a new statement appended to the Then branch after the Return becomes dead code, and one appended to the Else silently applies to only one of the two paths.
The following code illustrates the pattern detected by this rule:
Public Function Band(ByVal weightKg As Decimal) As String
' FLAGGED: Else clause follows an If branch that always leaves the block
If weightKg <= 1D Then
Return "letter"
Else
Return "parcel"
End If
End Function
Remediation
Remove the Else and dedent its body so it follows the If block, turning the condition into a guard clause: If weightKg ⇐ 1D Then Return "letter" followed by Return "parcel" at the procedure’s own level. The behaviour is identical and the nesting drops by one. Where the statement is Continue For, the same rewrite leaves the rest of the loop body unindented. If the If is part of an If … ElseIf chain in which every branch returns, keeping the chain is reasonable when the branches are parallel cases of one decision - in that case prefer a Select Case with a Case Else, which states the exhaustiveness explicitly instead of relying on the reader to notice that each branch returns.