5bff87abb4
* Modify rule S1479: allow statements with single-line clauses only * Improve compliant example * Improve description to match implementation * Address comments --------- Co-authored-by: Sebastien Marichal <sebastien.marichal@sonarsource.com>
133 lines
3.3 KiB
Plaintext
133 lines
3.3 KiB
Plaintext
== Why is this an issue?
|
|
|
|
When https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/statements/selection-statements#the-switch-statement[switch] statements have large sets of multi-line `case` clauses, the code becomes hard to read and maintain.
|
|
|
|
For example, the https://www.sonarsource.com/docs/CognitiveComplexity.pdf[Cognitive Complexity] is going to be particularly high.
|
|
|
|
In such scenarios, it's better to refactor the `switch` to only have single-line case clauses.
|
|
|
|
When all the `case` clauses of a `switch` statement are single-line, the readability of the code is not affected.
|
|
Moreover, `switch` statements with single-line `case` clauses can easily be converted into https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/operators/switch-expression[`switch` expressions], which are more concise for assignment and avoid the need for `break` statements.
|
|
|
|
=== Exceptions
|
|
|
|
This rule ignores:
|
|
|
|
* `switch` statements over `Enum` arguments
|
|
* fall-through cases
|
|
* `return`, `break` and `throw` statements in case clauses
|
|
|
|
== How to fix it
|
|
|
|
Extract the logic of multi-line `case` clauses into separate methods.
|
|
|
|
=== Code examples
|
|
|
|
The examples below use the "Maximum number of case" property set to `4`.
|
|
|
|
Note that from C# 8, you can use https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/operators/switch-expression[`switch` expression].
|
|
|
|
==== Noncompliant code example
|
|
|
|
[source,csharp,diff-id=1,diff-type=noncompliant]
|
|
----
|
|
public int MapChar(char ch, int value)
|
|
{
|
|
switch(ch) // Noncompliant
|
|
{
|
|
case 'a':
|
|
return 1;
|
|
case 'b':
|
|
return 2;
|
|
case 'c':
|
|
return 3;
|
|
// ...
|
|
case '-':
|
|
if (value > 10)
|
|
{
|
|
return 42;
|
|
}
|
|
else if (value < 5 && value > 1)
|
|
{
|
|
return 21;
|
|
}
|
|
return 99;
|
|
default:
|
|
return 1000;
|
|
}
|
|
}
|
|
----
|
|
|
|
==== Compliant solution
|
|
|
|
[source,csharp,diff-id=1,diff-type=compliant]
|
|
----
|
|
public int MapChar(char ch, int value)
|
|
{
|
|
switch(ch) // Compliant: All 5 cases are single line statements
|
|
{
|
|
case 'a':
|
|
return 1;
|
|
case 'b':
|
|
return 2;
|
|
case 'c':
|
|
return 3;
|
|
// ...
|
|
case '-':
|
|
return HandleDash(value);
|
|
default:
|
|
return 1000;
|
|
}
|
|
}
|
|
|
|
private int HandleDash(int value)
|
|
{
|
|
if (value > 10)
|
|
{
|
|
return 42;
|
|
}
|
|
else if (value < 5 && value > 1)
|
|
{
|
|
return 21;
|
|
}
|
|
return 99;
|
|
}
|
|
----
|
|
|
|
For this example, a `switch` expression is more concise and clear:
|
|
|
|
[source,csharp]
|
|
----
|
|
public int MapChar(char ch, int value) =>
|
|
ch switch // Compliant
|
|
{
|
|
'a' => 1,
|
|
'b' => 2,
|
|
'c' => 3,
|
|
// ...
|
|
'-' => HandleDash(value),
|
|
_ => 1000,
|
|
};
|
|
|
|
private int HandleDash(int value)
|
|
{
|
|
if (value > 10)
|
|
{
|
|
return 42;
|
|
}
|
|
else if (value < 5 && value > 1)
|
|
{
|
|
return 21;
|
|
}
|
|
return 99;
|
|
}
|
|
----
|
|
|
|
include::../resources-dotnet.adoc[]
|
|
|
|
* Microsoft Learn - https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/statements/selection-statements#the-switch-statement[The `switch` statement]
|
|
* Microsoft Learn - https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/operators/switch-expression[C#: Switch Expression]
|
|
|
|
include::../rspecator-dotnet.adoc[]
|
|
|