How to eliminate if-else with same condition but different actions inside another if-else?

Question

Suppose I have the code like following:

int age=(from user input);
int weight=(from user input);

if(age>60){
    if(weight>100){
        function1();
    }else if(weight<40){
        function2();
    }else{
        function3();
    }
}else if(age<20){
    if(weight>100){
        function4();
    }else if(weight<40){
        function5();
    }else{
        function6();
    }
}else{
    if(weight>100){
        function7();
    }else if(weight<40){
        function8();
    }else{
        function9();
    }
}

the problem is that the code pattern:

if(weight>100){
    //do something different
}else if(weight<40){
    //do something different
}else{
    //do something different
}

repeats as there are different ranges of age. And I can't enclose the inner if-else into a single function, because the things to do is different even through the condition is the same. Is there any way to modify this code so that the code pattern:

if(age>60){
    //do something different
}else if(age<20){
    //do something different
}else{
    //do something different
}

and

if(weight>100){
    //do something different
}else if(weight<40){
    //do something different
}else{
    //do something different
}

would appear once only?

Answer 1

Actually age and weight are 2 independent dimensions, all we need to do is to collect different combinations and then determine which function to call. So let's preset combinations in a map and just get the handler and call, not a single if/else is needed.

import java.util.*;
import java.util.function.Function;
import java.util.stream.Collectors;
import java.util.stream.IntStream;

public enum Section {

    AGE(20, 60),
    WEIGHT(40, 100);

    private int[] points;

    Section(int... points) {
        this.points = points;
    }

    public int indexOf(int value) throws Exception {
        List<Integer> points = Arrays.stream(this.points).boxed().collect(Collectors.toList());
        points.add(value);
        Collections.sort(points);
        return IntStream.range(0, points.size())
                .filter(index -> Objects.equals(points.get(index), value))
                .findFirst()
                .orElseThrow(() -> new Exception("Value not found"));
    }
}

class MyTest {
    private static Map<String, Function<Object, Object>> sectionHandlerMapper = new HashMap<>();

    private static Object function1(Object obj) {
        return "function1";
    }
    private static Object function2(Object obj) {
        return "function2";
    }
    private static Object function3(Object obj) {
        return "function3";
    }
    private static Object function4(Object obj) {
        return "function4";
    }
    private static Object function5(Object obj) {
        return "function5";
    }
    private static Object function6(Object obj) {
        return "function6";
    }
    private static Object function7(Object obj) {
        return "function7";
    }
    private static Object function8(Object obj) {
        return "function8";
    }
    private static Object function9(Object obj) {
        return "function9";
    }

    static {
        sectionHandlerMapper.put("2-2", MyTest::function1);
        sectionHandlerMapper.put("2-0", MyTest::function2);
        sectionHandlerMapper.put("2-1", MyTest::function3);
        sectionHandlerMapper.put("0-2", MyTest::function4);
        sectionHandlerMapper.put("0-0", MyTest::function5);
        sectionHandlerMapper.put("0-1", MyTest::function6);
        sectionHandlerMapper.put("1-2", MyTest::function7);
        sectionHandlerMapper.put("1-0", MyTest::function8);
        sectionHandlerMapper.put("1-1", MyTest::function9);
    }

    public static Object call(Object obj, int... indices) {
        String key = Arrays.stream(indices).boxed().map(String::valueOf).collect(Collectors.joining("-"));
        return sectionHandlerMapper.get(key).apply(obj);
    }

    public static void main(String[] args) throws Exception {
        int age = 19;
        int weight = 39;
        int ageIndex = Section.AGE.indexOf(age);
        int weightIndex = Section.WEIGHT.indexOf(weight);
        System.out.println(call("obj", ageIndex, weightIndex));
    }
}

As enum is used, it would be rather convenient to add more dimensions, just adding new enums, reconstructing 'sectionHandlerMapper' and modifying main() would work. It's easy to expand.

Answer 2

public class User {
    int age;
    int weight;
}

@FunctionalInterface
public interface WeightFunction {
    void perform(User user);
}

@FunctionalInterface
public interface AgeFunction {

    WeightFunction getWeightFunction(User user);
}

public class Age60Function implements AgeFunction {
    @Override
    public WeightFunction getWeightFunction(User user) {
        String desc = "Age 60 ";
        if (user.weight > 100) {
            return weightUser -> System.out.println(desc + "Age > 100");
        } else if (user.weight < 40) {
            return weightUser -> System.out.println(desc + "Age < 40");
        }
        return weightUser -> System.out.println(desc + "100 > Age > 40");
    }

}

Then you can create outer if and create related AgeFunction and perform the function.

Also another suggestion IF WEIGHT CATEGORY ARE CONSTANT FOR ALL AGE LIMITS, as this example weight category 100+, 40- and 100>age>40 you can follow below implementation.

public interface AdvanceAgeFunction {
    WeightFunction get100PlusWeightFunction(User user);
    WeightFunction get40MinusWeightFunction(User user);
    WeightFunction getWeightFunction(User user);
} 

Answer 3

Not particularly elegant. In brief, make up a method that do that inner-condition-check, and allow the "action" for different case to be passed in

// Change to use other proper functional interface.  Using Runnable
// just to align with your sample code.
doOnWeight(int weight, 
           Runnable overWeightAction,
           Runnable normalWeightAction,                       
           Runnable underWeightAction) {
    if(weight>100){
        overWeightAction.apply();
    }else if(weight<40){
        underWeightAction.apply();
    }else{
        normalWeightAction.apply();
    }
}    
//.......

// your original code, I assume your functions are instance methods
if(age>60){
    doOnWeight(weight, this::function1, this::function3, this::function2)
}else if(age<20){
    doOnWeight(weight, this::function4, this::function6, this::function5)
}else{
    doOnWeight(weight, this::function7, this::function9, this::function8)
}

Answer 4

I didn't completely get your question but from what I see it seems you wan't to lose your redundant code so here is what I suggest.

Create a temp variable suppose

int ageGroup;

and a function to compute the weight calculations where ageGroup is passed as a parameter

void CheckWeight(int ageGroup)
{
}

You can further reduce the redundant code by creating another function to manage your work done in function 1-9 in switch conditions.

Your final code blocks will look like this :

if(age>60){
    CheckWeight(1);
}else if(age<20){
    CheckWeight(2)
}else{
    CheckWeight(3)
}



void CheckWeight(int ageGroup)
{
    if(weight>100){
       Compute(ageGroup,1);
    }else if(weight<40){
       Compute(ageGroup,2);
    }else{
       Compute(ageGroup,3);
    }
}


void Compute(int ageGroup,int weightGroup)
{
    switch(ageGroup)
    {
        case 1: 
            switch(weightGroup)
            {
                case 1:
                    //func1;
                    break;
                case 2:
                    //func 2;
                    break;
                .
                .
            }    
            break;
       case 2:
       .
       .
       .
    }
}

This is the best I can come up with, there may be other implementations.

Answer 5

Without seeing what all your function1-9 do (which would probably be too much code for Stack Overflow anyway), it's hard to give a definitive answer. If function1, function2, and function3 all do some common thing when age > 60, and function1, function4, and function7 all do some common thing when weight > 100, then you might be able to split those functions apart, then put the common parts back together into new methods.

If you really have 9 different functions that have nothing in common, then there's no really good solution. You could try setting up a map that maps all 9 combinations to functions:

Map<String, Runnable> functions = new HashMap<>();
functions.put("senior_heavy") = this::function1;
functions.put("senior_medium") = this::function2;
functions.put("senior_light") = this::function3;
functions.put("middleaged_heavy") = this::function4;
...
functions.put("child_light") = this::function9;

[You'd have to think of better names, since a 20-year-old isn't exactly middleaged.]

Then:

String ageString;
// set this to "senior", "middleaged", or "child"
String weightString;
// set this to "heavy", "medium", or "light"

functions.get(ageString + "_" + weightString).run();

Runnable is the interface for methods that take no parameters and don't return a result. (For functions that take parameters and/or return results, there are other functional interface types that you'd use. Or you could define your own as in smac89's answer, which would let you give a more descriptive name to the functional interface type.) this::function1 is a reference to function1 belonging to the current object, this; if function1 is static, however, you'd need to say ThisClass::function1.

This would eliminate the duplicated if pattern, but it's not a pretty solution. If I were reviewing your code, I wouldn't complain much about your original code, since the alternatives aren't great either. (A 2-D array of Runnable is another alternative.)

Answer 6

The best I could come up with was roughly this:

interface WeightFunc {
    void function();
}

public static final List<WeightFunc> weightFuncs = Arrays.asList(weightFunc1, weightFunc2,...);

Now inside your method

List<WeightFunc> choices = null;

if(age>60){
    choices = Arrays.asList(weightFuncs.get(0), weightFuncs.get(1), ...);
    // 1, 2, 3
}else if(age<20){
    choices = Arrays.asList(weightFuncs.get(3), weightFuncs.get(4), ...);
    // 4, 5, 6
}else{
    choices = Arrays.asList(weightFuncs.get(6), weightFuncs.get(7), ...);
    // 7, 8, 9
}

WeightFunc choice = null;

if(weight>100){
    choice = choices.get(0);
    //do something different
}else if(weight<40){
    choice = choices.get(1);
    //do something different
}else{
    choice = choices.get(2);
    //do something different
}

Now call it:

choice.function();

IMO, I prefer your initial method to this