11< h2 > Why is this an issue?</ h2 >
22< p > Having two < code > cases</ code > in a < code > switch</ code > statement or two branches in an < code > if</ code > chain with the same implementation is at
3- best duplicate code, and at worst a coding error. If the same logic is truly needed for both instances, then in an < code > if</ code > chain they should
4- be combined, or for a < code > switch</ code > , one should fall through to the other.</ p >
5- < h3 > Noncompliant code example</ h3 >
6- < pre >
3+ best duplicate code, and at worst a coding error.</ p >
4+ < pre data-diff-id ="1 " data-diff-type ="noncompliant ">
75switch (i) {
86 case 1:
97 doFirstThing();
@@ -19,7 +17,8 @@ <h3>Noncompliant code example</h3>
1917 default:
2018 doTheRest();
2119}
22-
20+ </ pre >
21+ < pre data-diff-id ="2 " data-diff-type ="noncompliant ">
2322if (a >= 0 && a < 10) {
2423 doFirstThing();
2524 doTheThing();
@@ -35,9 +34,44 @@ <h3>Noncompliant code example</h3>
3534 doTheRest();
3635}
3736</ pre >
37+ < p > If the same logic is truly needed for both instances, then:</ p >
38+ < ul >
39+ < li > in an < code > if</ code > chain they should be combined </ li >
40+ </ ul >
41+ < pre data-diff-id ="2 " data-diff-type ="compliant ">
42+ if ((a >= 0 && a < 10) || (a >= 20 && a < 50)) { // Compliant
43+ doFirstThing();
44+ doTheThing();
45+ }
46+ else if (a >= 10 && a < 20) {
47+ doTheOtherThing();
48+ }
49+ else {
50+ doTheRest();
51+ }
52+ </ pre >
53+ < ul >
54+ < li > for a < code > switch</ code > , one should fall through to the other. </ li >
55+ </ ul >
56+ < pre data-diff-id ="1 " data-diff-type ="compliant ">
57+ switch (i) {
58+ case 1:
59+ case 3: // Compliant
60+ doFirstThing();
61+ doSomething();
62+ break;
63+ case 2:
64+ doSomethingDifferent();
65+ break;
66+ default:
67+ doTheRest();
68+ }
69+ </ pre >
70+ < p > When all blocks are identical, either this rule will trigger if there is no default clause or rule {rule:flex:S3923} will raise if there is a
71+ default clause.</ p >
3872< h3 > Exceptions</ h3 >
39- < p > Blocks in an < code > if</ code > chain that contain a single line of code are ignored, as are blocks in a < code > switch </ code > statement that contain a
40- single line of code with or without a following < code > break</ code > .</ p >
73+ < p > Unless all blocks are identical, blocks in an < code > if</ code > chain that contain a single line of code are ignored. The same applies to blocks in a
74+ < code > switch </ code > statement that contains a single line of code with or without a following < code > break</ code > .</ p >
4175< pre >
4276if (a == 1) {
4377 doSomething(); //no issue, usually this is done on purpose to increase the readability
@@ -47,14 +81,9 @@ <h3>Exceptions</h3>
4781 doSomething();
4882}
4983</ pre >
50- < p > But this exception does not apply to < code > if</ code > chains without < code > else</ code > -s, or to < code > switch</ code > -es without default clauses when
51- all branches have the same single line of code. In case of < code > if</ code > chains with < code > else</ code > -s, or of < code > switch</ code > -es with default
52- clauses, rule {rule:flex:S3923} raises a bug.</ p >
53- < pre >
54- if (a == 1) {
55- doSomething(); //Noncompliant, this might have been done on purpose but probably not
56- } else if (a == 2) {
57- doSomething();
58- }
59- </ pre >
84+ < h2 > Resources</ h2 >
85+ < h3 > Related rules</ h3 >
86+ < ul >
87+ < li > {rule:flex:S3923} - All branches in a conditional structure should not have exactly the same implementation </ li >
88+ </ ul >
6089
0 commit comments