When the same code is duplicated in two or more separate branches of a conditional, it can make the code harder to understand, maintain, and can
potentially introduce bugs if one instance of the code is changed but others are not.
Having two Cases
in the same Select
statement or branches in the same If
structure with the same
implementation is at best duplicate code, and at worst a coding error.
If a >= 0 AndAlso a < 10 Then
DoFirst()
DoTheThing()
ElseIf a >= 10 AndAlso a < 20 Then
DoTheOtherThing()
ElseIf a >= 20 AndAlso a < 50 ' Noncompliant; duplicates first condition
DoFirst()
DoTheThing()
Else
DoTheRest();
End If
Select i
Case 1
DoFirst()
DoSomething()
Case 2
DoSomethingDifferent()
Case 3 ' Noncompliant; duplicates case 1's implementation
DoFirst()
DoSomething()
Case Else:
DoTheRest()
End Select
If the same logic is needed for both instances, then:
- in an
If
structure they should be combined
If (a >= 0 AndAlso a < 10) OrElse (a >= 20 AndAlso a < 50) Then
DoFirst()
DoTheThing()
ElseIf a >= 10 AndAlso a < 20 Then
DoTheOtherThing()
Else
DoTheRest();
End If
- for a
Select
, the values should be put in the Case
expression list.
Select i
Case 1, 3
DoFirst()
DoSomething()
Case 2
DoSomethingDifferent()
Case Else:
DoTheRest()
End Select
Exceptions
Blocks in an If
chain or Case
clause that contain a single line of code are ignored.
If a >= 0 AndAlso a < 10 Then
DoTheThing()
ElseIf a >= 10 AndAlso a < 20 Then
DoTheOtherThing()
ElseIf a >= 20 AndAlso a < 50 ' no issue, usually this is done on purpose to increase the readability
DoTheThing()
End If
But this exception does not apply to If
chains without Else
-s, or to Select
-s without Case Else
clauses when all branches have the same single line of code. In the case of If
chains with Else
-s, or of
Select
-es with Case Else
clauses, rule S3923 raises a bug.
If a == 1 Then
DoTheThing() ' Noncompliant, this might have been done on purpose but probably not
ElseIf a == 2 Then
DoTheThing()
End If