hhariri / CleanCode

ReSharper Plugins
MIT License
122 stars 37 forks source link

Counting Switch-Case-Default all as one statement? #38

Open NW7US opened 6 years ago

NW7US commented 6 years ago

In my C# project, I have a switch statement that has many tens of values based on an Enum. This enum variable is used in a communications environment. Each case of this single switch statement handles, as you know, all of the possible values that enum variable stores.

I have a single method that is nothing but the switch-case-default statement. Each case sets roughly three variables and makes one call to another method which in turn is one of the values used to set one of those three variables.

In my mind, there is NO way to reduce this. The CleanCode extension seems to be counting each case statement, as well as the other statements.

It would seem to me that the counting should only include the 'switch' as a whole. Yes, statements made inside the block of code of a particular 'case' can be analyzed separately. But, seriously, this should not cause the "Method too long...responsibility" hint.

Example:

public enum SomeCoolEnum {

    STATE_VALUE_01 = 0,
    STATE_VALUE_02 = 1,
    ... snip ... for efficiency, I've not included all 63 values ...
    STATE_VALUE_63 = 63
}

public int someReturnValue(SomeCoolEnum theEnumValue) {
    int theReturnValue;
    switch (theEnumValue) {
        case STATE_VALUE_01:
        case STATE_VALUE_02:
        case STATE_VALUE_03:
        case STATE_VALUE_04:
        case STATE_VALUE_05:
            theReturnValue = callSomeSubRoutineToGetTheNewIntBasedOnFirstFive();
            break;
        case STATE_VALUE_06:
        case STATE_VALUE_07:
        case STATE_VALUE_08:
        case STATE_VALUE_09:
            theReturnValue = callSomeSubRoutineToGetTheNewIntBasedOnSecondFour();
            break;
        ... snip ... you get the idea
        default:
            callSomeSubRoutineToDealWithUnknowns();
    }
    return theReturnValue;
}
openshac commented 6 years ago

In Clean Code, Uncle Bob states that switch statements almost always break Single Responsibility and Open/Closed Principles (pg37-39)

"Even a switch statement with only two cases is larger than I’d like a single block or function to be"

There are plenty of articles online that elaborate on this, e.g.: https://stackoverflow.com/questions/505454/large-switch-statements-bad-oop https://www.c-sharpcorner.com/article/switch-statement-a-code-smell/ https://stackoverflow.com/a/7173795/283787

I don't know enough about your code and what is most appropriate but perhaps you could use an action dictionary instead.