How to condense large switch statements

Question

I have a large switch like following:

public int procList(int prov, ArrayList<TXValue> txValueList, Context context)
{
    switch(prov)
    {
    case Foo.PROV_ONE:
        return proc_one(txValueList, context);

    case Foo.PROV_NOE:
        return proc_noe(txValueList, context);

    case Foo.PROV_BAR:
        return proc_bar(txValueList, context);

    case Foo.PROV_CABAR:
        return proc_cabar(txValueList, context);

    case Foo.PROV_FAR:
        return proc_far(txValueList, context);

    case Foo.PROV_TAR:
        return proc_tar(txValueList, context);

    case Foo.PROV_LBI:
        return 408;

    default:
        return -1;
    }
}

In c++ I can use std::map<Foo, some_function_ptr> and use it in manner as following:

map[prov](txValueList, context);

There is not pointer to function in Java. However, it uses abstract classes like it is in the answer. So, is there a best way to eliminate huge switch clauses in java?

Cross-site duplicate: http://codereview.stackexchange.com/questions/42125/refactoring-large-switch-statement — Sotirios Delimanolis, Jun 15 '15 at 15:10
Then you could provide one method in your enum, which every value of it overrides. Example: http://stackoverflow.com/questions/14968075/enum-method-overriding — Zhedar, Jun 15 '15 at 15:17
@EpicPandaForce Probably the best solution without Java 8, but wasn't sure of the duplicate question issue though. — Zhedar, Jun 15 '15 at 15:27
the thing is, you need to populate the map, `map.put(Foo.PROV_ONE, this::proc_one);` etc. Is it really better than the switch block? — ZhongYu, Jun 15 '15 at 15:37
On a side note, you're violating Java's naming conventions. The code will be frowned upon by other developers. Drop the underscores in method names. Also, if you're switching on what looks like an enum, use enum and not int for the prov argument. Returning -1 as a default also looks like something taken straight from C++ and is sometimes considered to be a code smell. — JohnEye, Jun 15 '15 at 15:54

score 7 · Answer 1 · answered Jun 15 '15 at 15:22

What you're looking for is an enum.

public enum Prov {
    PROV_ONE(Foo.PROV_ONE) {
        @Override
        public int provMethod(List<TXValue> txValueList, Context context) {
            return proc_one(txValueList, context);
        }
    },
    PROV_NOE(Foo.PROV_NOE) {
        @Override
        public int provMethod(List<TXValue> txValueList, Context context) {
            return proc_noe(txValueList, context);
        }
    },
    PROV_BAR(Foo.PROV_BAR) {
        @Override
        public int provMethod(List<TXValue> txValueList, Context context) {
            return proc_bar(txValueList, context);
        }
    },
    PROV_CABAR(Foo.PROV_CABAR) {
        @Override
        public int provMethod(List<TXValue> txValueList, Context context) {
            return proc_cabar(txValueList, context);
        }
    },
    PROV_FAR(Foo.PROV_FAR) {
        @Override
        public int provMethod(List<TXValue> txValueList, Context context) {
            return proc_far(txValueList, context);
        }
    },
    PROV_TAR(Foo.PROV_TAR) {
        @Override
        public int provMethod(List<TXValue> txValueList, Context context) {
            return proc_tar(txValueList, context);
        }
    },
    PROV_LBI(Foo.PROV_LBI) {
        @Override
        public int provMethod(List<TXValue> txValueList, Context context) {
            return 408;
        }
    },
    UNKNOWN(Integer.MIN_VALUE) { // make sure this is not used in other IDs
                                 //decide if you actually need this
        @Override
        public int provMethod(List<TXValue> txValueList, Context context) {
            return -1;
        }            
    };

    private int id;

    private Prov(int id) {
        this.id = id;
    }

    public abstract int provMethod(List<TXValue> txValueList, Context context);

    public static Prov getById(int id) {
        for(Prov prov : Prov.values()) {
            if(id == prov.id) {
                return prov;
            }
        }
        //return null;
        //throw new IllegalArgumentException("Specified id was not found.");
        return Prov.UNKNOWN;
    }
}

Then you can do

public int procList(int prov, ArrayList<TXValue> txValueList, Context context)
{
    return Prov.getById(prov).provMethod(txValueList, context);
}

As `Foo` already is an `enum` in this case, you could even simplify it to `prov.provMethod(txValueList, context);` skipping the additional iteration. — Zhedar, Jun 15 '15 at 15:32
Sure; but is this really better than the switch block? Not criticizing your answer, but OP's requirement instead. — ZhongYu, Jun 15 '15 at 15:38
@Zhedar that would require changing `int prov` to `Prov prov`, but yes. — Nick Mertin, Jun 15 '15 at 15:40
@bayou.io that honestly depends on how often they need to use this switch case for every possible for element. If they use this in more than one place, then using enum subclass allows it so that if you add a new element, then you are forced to subclass it, and you have to modify it in one place in the project rather than all over the place. — EpicPandaForce, Jun 15 '15 at 15:46
@bayou.io It generally is, and the reasons it is better are: a. it enforces a common interface, b. it ensures at compile time that every option is covered, and c. if used in conjunction with an interface, it improves testability — biziclop, Jun 15 '15 at 15:51
@biziclop - generally :) but there are downside too. and this particular solution doesn't ensure at compile time that all `Foo.*` are covered anyway. If we make `Foo` an enum instead, OP may not want to introduce dependency from `Foo` to other codes. — ZhongYu, Jun 15 '15 at 15:59
@bayou.io That is true, if you use another enum, there's no such guarantee. — biziclop, Jun 15 '15 at 16:00
To be fair, the values in Foo are not enum, they are most likely `public static final int`. Normally I wouldn't have bound them with constructor parameter, but the int seems to be coming from somewhere. — EpicPandaForce, Jun 15 '15 at 16:02
But Kotlin's `when` statement (if assigning or there is a `let {}` at the end) are quite useful for this same use-case. — EpicPandaForce, Aug 15 '17 at 15:48

score 3 · Answer 2 · answered Jun 15 '15 at 16:05

3

To answer OP's question as it is framed -

Map<Integer, BiFunction<ArrayList<TXValue>,Context, Integer>> map = new HashMap<>();
{
    map.put(Foo.PROV_ONE, this::proc_one);
    // etc ....
    map.put(Foo.PROV_LBI, (x,y)->408 );
}

public int procList(int prov, ArrayList<TXValue> txValueList, Context context)
{
    if( ! map.contains(prov)) return -1;

    return map.get(prov).apply(txValueList, context);
}

answered Jun 15 '15 at 16:05

ZhongYu

19,446
5
33
61

Love it. If you can use Java 8, you should opt for this solution. – seanhodges Jun 15 '15 at 16:16
@seanhodges - why can't anyone use java8? :) - only in big corps. – ZhongYu Jun 15 '15 at 16:17
many people cannot use Java 8 for all sorts of reasons, but this answer is certainly the most appropriate if you can. – seanhodges Jun 15 '15 at 16:33
You can even say `return map.getOrDefault(prov, (l,c) -> -1).apply(txValueList, context);` without pre-checking via `map.contains`… – Holger Jun 19 '15 at 15:45

How to condense large switch statements

2 Answers2