0

TL;DR

What is the/a best practice approach to avoid if-else/switch statements when dealing with a calculator functionality that may expand at a later time (e.g. currently does addition/subtraction, room to add the modulo function without violating open/closed SOLID principle).

Background

I am working on a calculator program for a Udemy course assignment. The assignment asks us to modify an existing Calculator class to reduce the amount of if-else/switch statements. The original project was in Typescript, but I've been converting it to Java to see if I can recreate the functionality. Omitting some of the surrounding classes for brevity, the below is the App class that executes the calculator functions and prints the results.

public class App {
    public static void main(String[] args) throws Exception {

        Calculator calculator = new Calculator(0);

        calculator
        .execute(OperationType.add, 10)
        .execute(OperationType.add, 20)
        .execute(OperationType.subtract, 15)
        .execute(OperationType.multiply, 3)
        .execute(OperationType.divide, 2)
        .execute(OperationType.multiply, 1000);

        PrintHandler printer = new PrintHandler(calculator);
        printer.printResults();
    }

}

Output:

0.0 added to 10.0
10.0 added to 20.0
30.0 subtracted by 15.0
15.0 multiplied by 3.0
45.0 divided by 2.0
22.5 multiplied by 1000.0
-----------
Total: 22500.0

Question

While the code does work, the Calculator's execute method still has a large switch statement (code below) that routes to different class instantiation. I was attempting to simplify the execute method by abstracting out actual calculations (addition/subtraction) into separate classes, but I've basically just moved that logic around rather than bypassing the need for a switch statement. Since this model violates the open-closed solid principle, I'm wondering how I can implement a solution that follows solid principles and doesn't require the code to be re-opened in the event of adding a new operation type (e.g. Modulo).

public Calculator execute(OperationType operationType, float newValue) throws Exception {

    Operation newOperation;
    // Operation is the interface. SpecifiedOperations is the superclass holding the 
    // nested classes for each operation class, which house the logic for the "calculate" 
    // method. I could also see putting each operation class in its own class

    switch (operationType) {
        case add:
            newOperation = new SpecifiedOperations.AdditionOperation(currentValue, newValue);
            break;
        case subtract:
            newOperation = new SpecifiedOperations.SubtractionOperation(currentValue, newValue);
            break;
        case multiply:
            newOperation = new SpecifiedOperations.MultiplicationOperation(currentValue, newValue);
            break;
        case divide:
            newOperation = new SpecifiedOperations.DivisionOperation(currentValue, newValue);
            break;
        default:
            throw new Exception("Invalid operation type");
    }

    currentValue = newOperation.calculate();
    operations.add(newOperation);

    return this;
}

What I've Tried

In typescript I was able to somewhat achieve my goal via the code below. By mapping the operation type to the type of class, rather than an instance of it, I'm at least removing the need to open any methods and can just modify the map if new operation types come into the equation.

const operationGenerationMapper = new Map([
  [OperationType.addition, AdditionOperation],
  [OperationType.subtraction, SubtractionOperation],
  [OperationType.multiplication, MultiplicationOperation],
  [OperationType.division, DivisionOperation],
]);

export function generateOperation(
  operationType: OperationType,
  currentValue: number,
  newValue: number
): any {
  let newOperation: Operation;

  for (let [key, value] of operationGenerationMapper) {
    if (operationType == key) {
      return new value(currentValue, newValue);
    }
  }

  return null;
}

I found an interesting post titled Java way to create an object based on enum type that seems to be attempting to get at the same concept, but I'm not experienced enough with Java to modify the code they discussed/provided to my situation. I also wonder if using generics like this is considered "best practice" or just a way to answer the question provided.

I've also tried looking into factory patterns (https://www.youtube.com/watch?v=EdFq_JIThqM, https://www.youtube.com/watch?v=QNpwWkdFvgQ), but I haven't quite understood how (or if) those solutions would apply to my scenario.

Jwok
  • 646
  • 9
  • 23
  • 1
    An example related to the `enum` approach cited is seen [here](https://stackoverflow.com/a/37885601/230513). – trashgod Jul 26 '23 at 23:38
  • 1
    Why not make `calculate` a method on the `OperationType` enum, passing it `currentValue` and `newValue`? – tgdavies Jul 27 '23 at 01:19

2 Answers2

1

Thanks to trashgod & tgdavies for nudging me in the right direction. I finally wrapped my head around some of the extended capacities that enums are capable of and resolve my question.

This youtube video was helpful for me in understanding how to use fields can be used in enums. A little more research on the original post I referenced as well as what trashgod linked to helped me understand how to create abstract enum methods and how to overwrite them on enum values.

For those who are curious, I've posted the final code below. (Turns out I didn't even need the specific operator classes, as I could handle everything from the enum)

Calculator Class

public class Calculator {

    private ArrayList<Operation> operations = new ArrayList<Operation>();
    private float currentValue;

    public ArrayList<Operation> getOperations(){
        return operations;
    }

    public float getCurrentValue(){
        return currentValue;
    }

    public Calculator(float initialValue) {
        currentValue = initialValue;
    }

    // Because of the enum functionality, the switch statement
    // can be completely removed, simplifying the code as such.
    public Calculator execute(OperationType operationType, float newValue) {
 
        Operation newOperation = new Operation(this.currentValue, operationType, newValue);
        this.currentValue = newOperation.calculate();
        operations.add(newOperation);

        return this;
    }

}

Enums Class

public enum OperationType {
    add("added to") {
        public float calculate(float value1, float value2){
            return value1 + value2;
        }
    },
    subtract("subtracted from") {
        public float calculate(float value1, float value2){
            return value1 - value2;
        }
    },
    multiply("multiplied by") {
        public float calculate(float value1, float value2){
            return value1 * value2;
        }
    },
    divide("divided by") {
        public float calculate(float value1, float value2){
            return value1 / value2;
        }
    };

    // This is used for printing purposes down the line.
    // These are populated in the parenthesis next to each enum value
    public final String joinerText;

    // This constructor is necessary to make the above string field work
    OperationType(String joinerText){
      this.joinerText = joinerText;
    }

    // This declares the method for the enum, which each enum value 
    // sets to its own version
    public abstract float calculate(float value1, float value2);

}

Operation Class

public class Operation {
    private float value1;
    private float value2;
    private OperationType operationType;

    public Operation(float value1, OperationType operationType, float value2){
        this.value1 = value1;
        this.value2 = value2;
        this.operationType = operationType;
    }

    public float calculate(){
        return operationType.calculate(this.value1, value2);
    }

    public void printInfo(){
        System.out.println(value1 + " " + operationType.joinerText + " " + value2);
    }
}

For printing out results I used a separate printer method

public class PrintHandler {
    
    private Calculator calculator;

    public PrintHandler(Calculator calculator){
        this.calculator = calculator;
    }

    public void printResults(){

        ArrayList<Operation> operations = calculator.getOperations();

        for(Operation operation : operations){
            operation.printInfo();
        }

        System.out.println("-----------");
        System.out.println("Total: "+calculator.getCurrentValue());
    }
}

So final execution looks like this:

Calculator calculator = new Calculator(0);

calculator
.execute(OperationType.add, 10)
.execute(OperationType.add, 20)
.execute(OperationType.subtract, 15)
.execute(OperationType.multiply, 3)
.execute(OperationType.divide, 2)
.execute(OperationType.multiply, 1000);

PrintHandler printer = new PrintHandler(calculator);
printer.printResults();
Jwok
  • 646
  • 9
  • 23
-1

It's just my preference but you could instead define a functional interface that has an operation method, anytime you want to add a new operation type you just implement this interface with the operation as the function.

  • I might be wrong but I thought that's what I did. The `operation` interface defines a `calculate` function, and the various sub-classes then Calculator's `execute` method. I might be misunderstanding you though. Could you update your answer to include a mock code example? – Jwok Jul 26 '23 at 22:57