CODEWITHSUNDEEP
javaif-statementswitch-statement
Prusio
Prusio

Reputation: 425

Repetitive if statements reduction

What are the ways of reducing code quantity in a method that contains schematic conditional statements?

What would be a good practice of placing such code. Should it be in a different method/class?

Example code in Java. It contains one scheme repeated twice:

if (calculation[i].equals("*")) {
    if (stack.canDoOperation()) {
        times();
    } else {
        operationFailed = true;
    }
} else if (calculation[i].equals("+")) {
    if (stack.canDoOperation()) {
        sum();
    } else {
        operationFailed = true;
    }
}

Upvotes: 1

Views: 549

Answers (4)

tobias_k
tobias_k

Reputation: 82899

You could write a helper method, wrapping the given operation method into the commonly required checks. You could also put a try/catch in there, in case the operation can fail.

private boolean tryOperation(Runnable operation) {
    if (stack.canDoOperation()) {
        operation.run();
        return true;
    } else {
        return false;
    }
}

And use like this:

if (calculation[i].equals("*")) {
    operationFailed = ! tryOperation(this::times);
} else if (calculation[i].equals("+")) {
    operationFailed = ! tryOperation(this::sum);
}

Or, with Java 8, you can put the method references to those operations into a Map:

Map<String, Runnable> operations = new HashMap<>();
operations.put("*", this::times);
operations.put("+", this::sum);
...

Runnable operation = operations.get(calculation[i]);
if (operation != null && stack.canDoOperation()) {
    operation.run();
} else {
    operationFailed = true;
}

You can also combine the two approaches:

operationFailed = ! tryOperation(operations.get(calculation[i]);

Upvotes: 4

MC Emperor
MC Emperor

Reputation: 22977

Since Java 7, it's possible to switch over Strings.

switch (calculation[i]) {
    case "*":
        times();
        break;
    case "+":
        sum();
        break;
}

Furthermore, for each calculation type, you are repeating the canDoOperation check. It's better that you bubble up this check:

if (stack.canDoOperation()) {
    operationFailed = true;
}
else {
    switch ... // Your switch statement
}

I suggest you move these operations to a separate method:

/**
 * Performs the specified operation.
 * @param The operation to perform.
 * @return True if the operation was succeeded, false otherwise.
 */
boolean performOperation(String operation) {
    if (!stack.canDoOperation()) {
        return false;
    }
    switch (operation) {
        case "*":
            times();
            break;
        case "+":
            sum();
            break;
        default:
            return false;
    }
    return true;
}

However, without knowing the rest of your code, it's unclear whether this is a good solution.

Upvotes: 0

azro
azro

Reputation: 54148

You can use a switch statement into an if and maybe in a more logical way : check if you can do an operation, and if so find which one : 

if (stack.canDoOperation()) {
    switch(calculation[i]){
        case "*": times(); break;
        case "+": sum();   break;
} else {
    operationFailed = true;
}

You can also give the value of operationFailed into the if statement :

  1. operationFailed = !stack.canDoOperation() will be done first (if you cannot do an operation, operationFailed will true;
  2. the value of !operationFailed (!!stack.canDoOperation() = stack.canDoOperation()) will be used in the if test
if (!(operationFailed = !stack.canDoOperation())) {
    switch(calculation[i]){
        case "*": times(); break;
        case "+": sum();   break;
}

Upvotes: 0

Michał Bil
Michał Bil

Reputation: 3589

Here is some example of how you can utilize repetitive statements. For more clarity you can extract stack.canDoOperation() to variable if it doesn't break your logic. Also you can consider moving it to assertion in beginning of the method and than put calculation[i] into switch statement.

if (calculation[i].equals("*") && stack.canDoOperation()) {
        times();
    }
} else if (calculation[i].equals("+")  && stack.canDoOperation()) {
        sum();
    }
} else {
        operationFailed = true;
}

Upvotes: 0

Related Questions