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 clauses in a when statement or two branches in an if chain with the same implementation is at best duplicate
code, and at worst a coding error.
if (a >= 0 && a < 10) {
doFirstThing()
doTheThing()
}
else if (a >= 10 && a < 20) {
doTheOtherThing()
}
else if (a >= 20 && a < 50) { // Noncompliant; duplicates first condition
doFirstThing()
doTheThing()
}
else {
doTheRest()
}
when (x) {
1 -> {
doFirstThing()
doSomething()
}
2 -> doSomethingDifferent()
3 -> { // Noncompliant; duplicates case 1's implementation
doFirstThing()
doSomething()
}
else -> doTheRest()
}
If the same logic is needed for both instances, then:
- in an
if structure they should be combined
if ((a >= 0 && a < 10) || (a >= 20 && a < 50)) {
doFirstThing()
doTheThing()
}
else if (a >= 10 && a < 20) {
doTheOtherThing()
}
else {
doTheRest()
}
- for a
when, the values should be put together in the branch expression list.
when (x) {
1, 3 -> {
doFirstThing()
doSomething()
}
2 -> doSomethingDifferent()
else -> doTheRest()
}
Exceptions
Blocks in an if chain or in a when branch that contain a single line of code are ignored.
if (a == 1) {
doSomething() //no issue, usually this is done on purpose to increase the readability
} else if (a == 2) {
doSomethingElse()
} else {
doSomething()
}
But this exception does not apply to if chains without else-s, or to when-es without else
clauses when all branches have the same single line of code. In case of if chains with else-s, or of when-es
with default clauses, rule S3923 raises a bug.
if (a == 1) {
doSomething() // Noncompliant, this might have been done on purpose but probably not
} else if (a == 2) {
doSomething()
}
