Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
NW7US opened this issue Jul 26, 2018 · 1 comment
Open

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

NW7US opened this issue Jul 26, 2018 · 1 comment

Comments

@NW7US
Copy link

NW7US commented Jul 26, 2018

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
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants