Is there a way to simplify this switch-case?

Question

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.

score 4 · Accepted Answer · answered Jan 03 '16 at 19:42

4

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).

answered Jan 03 '16 at 19:42

Danny_ds

11,201
1
24
46

1

Because of the contrived example it is hard to come up with a better answer than this one. You have some logic which is specific to certain letters - that belongs in a switch/case. You have additional logic which depends on whether the letter is a vowel; that should be handled outside the switch block. – Carl Manaster Jan 03 '16 at 20:08

score 1 · Answer 2 · edited May 23 '17 at 11:44

1

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

edited May 23 '17 at 11:44

Community

1
1

answered Jan 03 '16 at 19:35

Niklas

955
15
29

score 1 · Answer 3 · answered Jan 03 '16 at 20:43

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.

score 0 · Answer 4 · answered Jan 03 '16 at 19:42

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?

Is there a way to simplify this switch-case?

4 Answers4