CODEWITHSUNDEEP
c#switch-statementrefactoring
vrwim
vrwim

Reputation: 14380

Is there a way to simplify this switch-case?

Like the title says, I want to simplify a switch-case statement. I currently have this in my switch-case statement:

switch(someEnum) {
case EnumType.A:
    SomeMethodSpecificToA();
    AMethodIShouldCallOnAllVowels();
    break;
case EnumType.B:
case EnumType.C:
case EnumType.D:
    SomeMethodSpecificToTheseThreeLetters();
    AMethodIShouldCallOnAllConsonants();
    break;
case EnumType.E:
    SomeMethodSpecificToE();
    AMethodIShouldCallOnAllVowels();
    break;
// All other letters, also containing the vowels & consonants methods
}

So I know I can chain multiple case statements to make them do the same thing, but I don't know how I can make it so that 2 letters do 2 separate things and then fall through to the second statement, for all vowels (or all consonants). In Swift I would do something like this:

func test(someEnum: EnumType) {
    switch someEnum {
    case .A:
        someMethodSpecificToA()
        fallthrough
    case .B, .C, .D:
        someMethodSpecificToTheseThreeLetters()
        fallthrough
    case .E:
        someMethodSpecificToE()
        fallthrough
    case .A, .E:
        aMethodIShouldCallOnVowels()
    case .B, .C, .D:
        aMethodIShouldCallOnAllConsonants()
    }
}

Is there a way to do this without using 2 switch statements? It seems redundant as I already switched on that variable.

Upvotes: 1

Views: 632

Answers (4)

drf
drf

Reputation: 8699

Is there a way to do this without using 2 switch statements?

Yes. Use if statements.

EnumType[] Vowels = new [] {EnumType.A, EnumType.E, EnumType.I, EnumType.O, EnumType.U};

if (someEnum == EnumType.A)
    SomeMethodSpecificToA();

if (new [] {EnumType.B, EnumType.C, EnumType.D}.Contains(someEnum))
    SomeMethodSpecificToTheseThreeLetters();

if (someEnum == EnumType.E)
    SomeMethodSpecificToE();

if (Vowels.Contains(someEnum))
    AMethodIShouldCallOnAllVowels();

Depending on the complexity of the 'letters' in your actual code, you may find classes rather than enums better suited. In doing so, you can replace all conditional logic (if and switch statements). One refactoring option may be something like the below:

abstract class Letter
{
    public char Value { get; private set; }
    protected abstract void FrobInternal();
    public void Frob()
    {
        FrobInternal();
        // optionally code to be called for all letters
    }

    // private constructor limits inheritance to nested classes
    private Letter(char value) { Value = value; }

    class Vowel : Letter
    {
        public Vowel(char letter) : base(letter) { }
        sealed protected override void FrobInternal()
        {
            FrobVowel();
            AMethodIShouldCallOnAllVowels();
        }
        protected virtual void FrobVowel() { }
        private void AMethodIShouldCallOnAllVowels()
        {
            // Implementation...
        }
    }
    class Consonant : Letter
    {
        public Consonant(char letter) : base(letter) { }
        sealed protected override void FrobInternal()
        {
            FrobConsonant();
            AMethodIShouldCallOnAllConsanants();
        }
        protected virtual void FrobConsonant() { }
        private void AMethodIShouldCallOnAllConsanants()
        {
            // Implementation...
        }
    }

    class ConsonantBCD : Consonant
    {
        public ConsonantBCD(char letter) : base(letter) { }
        protected override void FrobConsonant()
        {
            // Special implemenation for B, C, D
        }
    }

    class LetterA : Vowel
    {
        public LetterA() : base('A') { }
        protected override void FrobVowel()
        {
            // Special implementation for A
        }
    }
    class LetterE : Vowel
    {
        public LetterE() : base('E') { }
        protected override void FrobVowel()
        {
            // Special implementation for E
        }
    }

    // use public readonly fields to replicate Enum functionality
    public static readonly Letter A = new LetterA();
    public static readonly Letter B = new ConsonantBCD('B');
    public static readonly Letter C = new ConsonantBCD('C');
    public static readonly Letter D = new ConsonantBCD('D');
    public static readonly Letter E = new LetterE();
    public static readonly Letter F = new Consonant('F');
    // ...   
    public static readonly Letter Z = new Consonant('Z');
}

And then you could replace the prototype function above with simply:

void Test(Letter l) {
    l.Frob();
}

The above refactoring is merely one option for a closed set of values to emulate an enumeration. The strategy or visitor patterns may also be useful to consider.

Upvotes: 1

Danny_ds
Danny_ds

Reputation: 11406

I would simply limit the cases to the Specific()'s, and after the switch block place a simple if-else:

if IsVowel
    AMethodIShouldCallOnAllVowels();
else
    AMethodIShouldCallOnAllConsonants();

Also check out default: (but probably of no use in this case).

Upvotes: 4

xtofl
xtofl

Reputation: 41519

When I have to understand what happens when case A is executed, I don't want to scroll down the whole switch to find out if A occurs more than once.

Are you really ending up in a better maintainable program after that refactoring? Wouldn't the KISS solution be to just group 'all stuff for A' after the A label?

Upvotes: 0

Niklas
Niklas

Reputation: 1000

Going to the next case is "goto Case 2;". i also believe this is a related question : [Using `continue` keywoard in a switch nest inside a foreach loop

Upvotes: 1

Related Questions