Minimise if else conditions in method

Question

I have the following class.

public enum EnumService
{
    ONE, TWO, THREE, FOUR,.............HUNDRED;
    //Values till HUNDRED

    public static EnumService returnMockService(String request)
    {
        //some string match
        if(request.matches("/abc*")){
           return ONE;
        } 
        //some other string is match      
        else if(request.matches("/bcd*"))
            return TWO;
        else if(request.matches("/decf*"))
            return THREE;
        //many else if conditions
        else if(request.matches("/wxyz*"))
            return HUNDRED;
        return null;     

    }


}

The code is not standard with more if else statements.

I want to minimize the number of if calls in above method yet maintaining the return type as EnumService

Any better option to do this.

It would be great if some can help me in making it clean.

@user2310289. I just want to check if some string is matched, return particular ENUM. This is just sample data. — Patan, Mar 12 '14 at 08:21
You can remove all `else` statements since they are obsolete because your `if` statements have `return` statements. — Ceiling Gecko, Mar 12 '14 at 08:23
@CeilingGecko even if there would be no `return` statements, `else` is not necessary because conditions are all disjoint — mangusta, Mar 12 '14 at 08:28

fge · Accepted Answer · 2014-03-12T08:29:29.700

First: no need to else if you return.

Second: you can optimize it a LOT if you use this string as a parameter in the enum:

public enum EnumService
{
    ONE("abc*"),
    // etc

    private static final Map<Pattern, EnumService> MAPPING
        = new HashMap<>();

    static {
        for (final EnumService svc: values())
            MAPPING.put(svc.pattern, svc);
    }

    private final Pattern pattern;

    EnumService(final String s)
    {
        pattern = Pattern.compile(s);
    }

    public static EnumService returnMockService(String request)
    {
        for (final Map.Entry<Pattern, EnumService> entry: MAPPING.entrySet())
            if (entry.getKey().matcher(request).matches())
                return entry.getValue();
        return null;
    }
}

T.J. Crowder · Answer 2 · 2014-03-12T08:29:37.980

I would put the strings that you're matching along with the EnumService value they should map to into an array of simple objects, then loop through the array.

E.g.:

ArrayEntry[] entries = new ArrayEntry[] {
    new ArrayEntry("/abc*", EnumService.ONE),
    // ...and so on...
};

and then:

for (ArrayEntry entry : entries) {
    if (request.matches(entry.str)) {
        return entry.value;
    }
}
return null;

...where ArrayEntry is just a simple class with those two properties (str and value).

If you don't want to have ArrayEntry, you can use a Map:

Map<String,EnumService> entries = new HashMap<String,EnumService>();
entries.put("/abc*", EnumService.ONE);
//...and so on...

and then

for (Map.Entry<String,EnumService> entry : entries.entrySet()) {
    if (request.matches(entry.getKey())) {
        return entry.getValue();
    }
}
return null;

Or you can do it with parallel arrays instead:

String[] strings = new String[] { "/abc*", /*...and so on...*/ };
EnumService[] values = new EnumService[] { EnumService.ONE, /*...and so on...*/ };

and then

int n;
for (n = 0; n < strings.length; ++n) {
    if (request.matches(strings[n])) {
        return values[n];
    }
}
return null;

But parallel arrays tend to be a bit of a maintenance issue.

Any reason you've avoided the `Map` and gone with `Parallel Arrays`? — Dan Temple, Mar 12 '14 at 08:25
@DanTemple: An insufficiency of coffee is the only thing I can come up with. :-) Thanks, I've added it. — T.J. Crowder, Mar 12 '14 at 08:27

score 1 · Answer 3 · answered Mar 12 '14 at 08:23

1

You can go for design patterns, best for these kind of things are state pattern. State Pattern is to solve this kind of issues and make the code more compatible & flexible.
Look over this http://www.javacodegeeks.com/2013/08/state-design-pattern-in-java-example-tutorial.html

answered Mar 12 '14 at 08:23

Karthik Surianarayanan

630
3
21

Surely this is the [Strategy](http://en.wikipedia.org/wiki/Strategy_pattern) pattern. The [State](http://en.wikipedia.org/wiki/State_pattern) pattern would imply a change to the state of the enums which is not happening here. – OldCurmudgeon Mar 12 '14 at 08:29
@OldCurmudgeon Strategy pattern is really about having a different implementation that accomplishes the same thing. But State pattern is about doing different things based on the state. Here he is having different states, and based on the states he is doing things. Make it simple. – Karthik Surianarayanan Mar 12 '14 at 08:36

Scary Wombat · Answer 4 · 2014-03-12T08:29:16.143

0

If there is no logic to map /abc to ONE and /bcd to TWO etc, then maybe you need to load these into an array or map, then simply get the index of the array where it matches.

myMap.put("abc", ONE);
myMap.put("bcd", TWO);

edited Mar 12 '14 at 08:29

answered Mar 12 '14 at 08:22

Scary Wombat

44,617
6
35
64

score 0 · Answer 5 · edited May 23 '17 at 12:13

0

Have a look at the below link.

Why can't I switch on a String?

As stated there, if you are using jdk 7, just use switch on the strings. If not, create an enum of /abc*,/bcd*,etc. and use them in switch.

Hope this helps.

Or you could always, store them in an array and loop through them. That would be much easier but costs an extra array.

edited May 23 '17 at 12:13

Community

1
1

answered Mar 12 '14 at 08:23

anirudh

4,116
2
20
35

a switch statement of a hundred is still going to be awful – Scary Wombat Mar 12 '14 at 08:26
yeah.. then use an array, would be much simpler. – anirudh Mar 12 '14 at 08:27
The problem here (not my downvote) is that you can't use `request.matches` with the cases of the `switch`. – T.J. Crowder Mar 12 '14 at 08:52

Minimise if else conditions in method

5 Answers5