Using state pattern to remove if else statements

Question

I need to get rid of if, else statements from my code as it is a smell. I know you can implement the state pattern to get rid of them, however in my code I'm checking if the string input is a certain operator and then doing things on a stack. How would I go about this? Should I make different states which do the maths and the stack operations as well? My main problem is I don't understand how to implement it without using a if statement, for example if I encounter a operator I want to do something, but as theres a number of different operators so I need to check that the input is a "+" to carry out the code required for that section.

Heres my code:

package src;

import java.util.Arrays;
import java.util.List;
import java.util.Stack;

public class RPNCalculator {

    public int solveEquation(final String input) {
        final Stack<Integer> operands = new Stack<Integer>();

        List<String> tokenizedInput = Arrays.asList(input.split(" "));

        for (String currentToken : tokenizedInput) {
            try {
                int inputAsInt = Integer.parseInt(currentToken);
                operands.push(inputAsInt);
                continue; //
            } catch (NumberFormatException exception) {

            }

            if (currentToken.contains("+")) {
                int total = operands.pop() + operands.pop();
                operands.push(total);

            } else if (currentToken.contains("-")) {
                int secondNum = operands.pop();
                int total = operands.pop() - secondNum;
                operands.push(total);

            } else if (currentToken.contains("*")) {
                int total = operands.pop() * operands.pop();
                operands.push(total);

            } else if (currentToken.contains("/")) {
                int secondNum = operands.pop();
                int total = operands.pop() / secondNum;
                operands.push(total);

            }

With your operator checks you should be definitive. Otherwise what happens when you receive the token "*/-+"? Is that a multiply, a divide, a minus, or add? — Kain0_0
– Kain0_0, Commented Dec 6, 2019 at 1:16
The one true smell in your example code is that try/continue/catch block., That took me far too long to decipher. The if/else's simply contain a lot of duplicated/inconsistent code and that code could be improved. But testing via that series of if's isn't a smell in itself. — David Arno
– David Arno, Commented Dec 6, 2019 at 8:34

ShapeOfMatter · Accepted Answer · 2019-12-06 00:43:24Z

I don't think if(){}else{} smells per se.

What suggests there could be improvement is that the four block bodies are so similar. continue is also maybe a bit stinky.

Probably what you want is a dictionary from operator-string identifiers to usable operator objects. The State Design Pattern seems like a work-around when your language doesn't support functional patterns. Java has functional interfaces, which are like a compromise.

static Dictionary<String, BinaryOperator<int>> operators = ... ;
...

BinaryOperator<int> op = operators.get(currentToken);

if(null == op){
    operands.push(Integer.parseInt(currentToken));
} else {
    operands.push(op.apply(operands.pop(), operands.pop()));
}

As evidence that the if isn't the problem: Consider that we could replace the above contents of operands.push() with a ternary operator instead of using if...else. But that would obscure the fact that the two blocks do different things: The if grows the stack and the else shrinks it.

You'll notice that I left out the try{}catch. Generally people don't like it when exceptions drive non-exceptional behavior. Depending on your spec, you probably don't need it. Any input token is either an int, an operator, or invalid.

I've reduced the if else statements by applying this, but instead I used a hashmap and a interface that assigned all the operators. Like this example you've gave me I've got a for loop to read the data and one if,else statement, does this get rid of the smell? I would have liked to get rid of this if, else statement as well however it seems impossible so it cant be a smell can it? — Harv
– Harv, Commented Dec 6, 2019 at 11:20
When we talk about a "smell", we just mean something that suggests there might be a better way of performing the task or representing the computation. It's not a problem in-and-of itself; don't stress about it too much. if(){}else{} is totally appropriate in a lot of cases. — ShapeOfMatter
– ShapeOfMatter, Commented Dec 6, 2019 at 14:37

DetriusXii · Accepted Answer · 2020-02-02 14:47:31Z

0

The Option monad and the either monad are monads suited to removing the amount of if statements. The state monad is unrelated to if else expressions. The state monad is related to thread a common parameter across multiple functions. Without tail call optimization, (the compiler releases the caller's stack before executing the callee) the state monad can also create stack overflow.

answered Feb 2, 2020 at 14:47

DetriusXii

1651 silver badge5 bronze badges

Add a comment |

Stack Exchange Network

Using state pattern to remove if else statements

2 Answers 2

Hot Network Questions

Using state pattern to remove if else statements

2 Answers 2

Related

Hot Network Questions