Skip to main content
Commonmark migration
Source Link

Review:

###Review: FromFrom extensibility and in pro of having the possibility of including in future versions more operations, it is a good approach. The code is very simple, and it is easy to read so it is very good your design proposal.

###An alternative

An alternative

###Review: From extensibility and in pro of having the possibility of including in future versions more operations, it is a good approach. The code is very simple, and it is easy to read so it is very good your design proposal.

###An alternative

Review:

From extensibility and in pro of having the possibility of including in future versions more operations, it is a good approach. The code is very simple, and it is easy to read so it is very good your design proposal.

An alternative

Source Link

###Review: From extensibility and in pro of having the possibility of including in future versions more operations, it is a good approach. The code is very simple, and it is easy to read so it is very good your design proposal.

The main purpose of applying design patterns is to simplify things, to reach the maximum level of abstraction and allow you to write meaningful code, not just repeat stuff

So, you did good.

However, there are some observations:

package oopdesign.calculator;

//Singleton is a good approach for this problem
public class Calculator {

    //By default any object is null
    //Do not put it as public, you have the getInstance method
    private static Calculator instance;

    //You are limiting the operations to handle
    CalculationStrategy calculationStrategy;

    //This is not a Singleton if you allow the default constructor (its public by default)
    private Calculator() {
    }

    public void setCalculationStrategy(CalculationStrategy calculationStrategy) {
        this.calculationStrategy = calculationStrategy;
    }

    public static Calculator getInstance() {
        if (instance == null)
            instance = new Calculator();
        return instance;
    }

    //You should think about handle the most general data type (this case double)
    public double calculate(double value1, double value2) {
       return calculationStrategy.calculate(value1, value2);
    }
}
package oopdesign.calculator;

public class CalculatorMain {

    public static void main(String[] args) {

        Calculator c = Calculator.getInstance();

        //There is a problem with it, you need to instanciate the strategies
        //each time you need to use it
        c.setCalculationStrategy(new AdditionStrategy());
        System.out.println(c.calculate(5,2));

        //It requires space, plus you are not being efficient by storing
        //there operations (calculation strategies)
        c.setCalculationStrategy(new SubtractionStrategy());
        System.out.println(c.calculate(5,2));

        c.setCalculationStrategy(new MultiplicationStrategy());
        System.out.println(c.calculate(5,2));

        c.setCalculationStrategy(new DivideStrategy());
        System.out.println(c.calculate(5,2));
    }
}

###An alternative

import java.util.HashMap;
import java.util.Map;

public class Calculator {

    private static Calculator instance;

    //search in Constant time (approximately)
    private Map<String, CalculationStrategy> calculationStrategies;

    private Calculator() {
        calculationStrategies = new HashMap<>();
    }

    public void addCalculationStrategy(String name, CalculationStrategy strategy) {
        calculationStrategies.put(name, strategy);
    }

    public static Calculator getInstance() {
        if (instance == null)
            instance = new Calculator();
        return instance;
    }

    //double b... means that there may be 0 to n parameters
    //consider that there are unitary operators or functions in a calculator
    public double calculate(String name, double a, double... b) {
        return calculationStrategies.get(name).calculate(a, b);
    }
}
package oopdesign.calculator;

public class Main {

    public static void main(String[] args) {
        Calculator calculator = Calculator.getInstance();

        //Use a lambda instead
        calculator.addCalculationStrategy("+", (a, b) -> a + b[0]);
        //[b] is taken as an array but is a variadic parameter
        calculator.addCalculationStrategy("-", (a, b) -> a - b[0]);
        calculator.addCalculationStrategy("*", (a, b) -> a * b[0]);
        calculator.addCalculationStrategy("/", (a, b) -> a / b[0]);
        calculator.addCalculationStrategy("Abs", (a, b) -> Math.abs(a));
        calculator.addCalculationStrategy("Cos", (a, b) -> Math.cos(a));
        calculator.addCalculationStrategy("Sin", (a, b) -> Math.sin(a));

        System.out.println(calculator.calculate("+", 1, 3));
        System.out.println(calculator.calculate("-", 1, 3));
        System.out.println(calculator.calculate("*", 1, 3));
        System.out.println(calculator.calculate("/", 1, 3));
        System.out.println(calculator.calculate("Abs", -66));
        System.out.println(calculator.calculate("Cos", 75));
        System.out.println(calculator.calculate("Sin", 28));
        System.out.println(calculator.calculate("+", 666, 777));
    }
}

About double b... read this post about Variadic function parameters, as I said, it is a way to have multiple parameters, From 0 To N parameters

Thanks for reading this answer.