Java OOP Calculator (no GUI)

Question

i have created an object orientated Calculator without GUI.

i have provided only the most important parts of the code. I want to add another review question for those parts (especially: MathResult and Operands)

Interface Calculator

public interface Calculator {
    MathResult<?> calculate(String operation);
}

Interface OperationParser

public interface OperationParser<T> {
    boolean matches(String operation);
    Operation<T> getOperation();
}

example-implementation: SimpleCalculator

public class SimpleCalculator implements Calculator {

    private final List<OperationParser<?>> operationParsers = new ArrayList<>();

    public SimpleCalculator(){
        operationParsers.add(new SimpleInfixArithmeticOperationParser(MathContext.DECIMAL64));
        operationParsers.add(new SimpleTrigonometricOperationParser());
    }

    @Override
    public MathResult<?> calculate(String input) {
        Optional<OperationParser<?>> parser = operationParsers.stream().filter(ip -> ip.matches(input)).findAny();
        if(parser.isPresent()){
            System.out.println("calculating "+parser.get().getOperation());
            return parser.get().getOperation().calculate();
        }else {
            throw new IllegalArgumentException("no valid parser for input '"+input+"' found");
        }
    }

}

Math Result

public class MathResult<T> {

    private final T t;

    public MathResult(T t) {
        this.t = t;
    }

    @Override
    public String toString() {
        String type = t == null ? "null" : t.getClass().getSimpleName();
        return "MathResult{" + type + ':' + t + '}';
    }
}

Operation

public class Operation<T> {

    public final Operands<T> operands;
    public final String operationSymbol;
    public final Function<Operands<T>, MathResult<T>> function;

    public Operation(Operands<T>  operands,String operationSymbol, Function<Operands<T>, MathResult<T>> function){
        this.operands = operands;
        this.operationSymbol = operationSymbol;
        this.function = function;
    }

    public MathResult<T> calculate() {
        return function.apply(operands);
    }

}

Operands

public class Operands<T> {

    private final T first;
    private final T second;

    public Operands(T single) {
        this(single, null);
    }

    public Operands(T first, T second) {
        if (first == null) {
            throw new IllegalArgumentException("first (single) operand must be set");
        }
        this.first = first;
        this.second = second;
    }

    public T get() {
        return getFirst();
    }

    public T getFirst() {
        return first;
    }

    public T getSecond() {
        if (second == null) {
            throw new IllegalArgumentException("second Operand does not exist");
        }
        return second;
    }
}

main Class

public class CalculatorApp {

    public static void main(String[] args){
        final Calculator calculator = new SimpleCalculator();
        final Scanner scanner = new Scanner(System.in);
        final Pattern exitPattern = Pattern.compile("[eE][xX][iI][tT]");
        while (true) {
            String line = scanner.nextLine();
            if(exitPattern.matcher(line).matches()){
                break;
            }
            try {
                MathResult<?> result = calculator.calculate(line);
                System.out.println("result: " + result);
            }catch (Exception e){
                System.out.println("error: " + e);
            }
        }

    }
}

Answer 1

Naming

What you call "operation", in most cases I'd rename to be an "expression".

Documentation

You create a quite complex set of classes and interfaces, and the only hint about their usage is in the class and method names. You should add Javadoc comments at least to public classes, interfaces and methods.

Interface OperationParser

public interface OperationParser<T> {
    boolean matches(String operation);
    Operation<T> getOperation();
}

getOperation() doesn't have parameters, so where does it get its input (expression text form) from? Probably, you rely on it being called only after a matches() call, and save the expression internally in some field. This goes against the expectations of callers, and will make any multi-threaded use of the parser impossible.

Generics

You often use a type variable T, but it's not clear what it's meant to be (for lack of Javadoc). My best guess is that it might be the number type you deal with, expecting e.g. Integer or Double, and then I'd recomment to specify that like <T extends Number>.

In the Operation class, you assume both operands as well as the result to be of the same type. What about dviding two integers? In math (as opposed to many programming languages), this gives a rational number, and generally not an integer.

User interface

In the SimpleCalculator.calculate() method, you print to System.out. If that's meant for debugging, use some logging framework instead (a basic one comes with the JRE: the java.util.logging package).

If it's really meant for the user, restructure your code. It's not some calulate() method's responsibility to tell something to a user. A calulate() method should calculate things and return the calculation results to the caller.

Answer 2

Naming

MathResult; I was wondering why the prefix Math in MathResult is important. Are there other types of results? If not, we can keep it simple and only name it Result.

Operation; As @Ralf Kleberhoff already points out, would Expression a better fit. The grouping of operations and operands is an expression, where an operation is a function such as addition, subtraction and so on, and an operand is a number.

OperationParser; A better fit would be ExpressionParser, like described in Operation.

Generics

In addition to @Ralf Kleberhoff, I would like to point out that operands can only have the same type. The Expression 1 + 1.5 is not possible.

(Every operand could be a Double and to display an Integer can be the work of the UI-Logic)

The Calculator

public interface Calculator {
   MathResult<?> calculate(String operation);
}

The calculator is bound to String. Instead, you can use a custom data type that abstracts String. The abstraction is already in your code: Operation (which we will call Expression):

public interface Calculator {
    Result calculate(Expression expression);
}

This method signature makes more sense: A calculator calculates a result based on an expression.

The Simple Calculator

After we changed the method signature of Calculator the SimpleCalculator will change, too.

Without the change the SimpleCalculator has four responsibilities:

MathResult<?> calculate(String input) { 
  Optional<OperationParser<?>> parser = // 1. Parse the string input
  if(parser.isPresent()){
      System.out.println(/* 2. print message */)
      return // 3. calculate the result
  }else {
      // 4. handle wrong input
  }
}

Just as a hint, we could use the methods on Optional to get a code like:

MathResult<?> calculate(String input) { 
    Result result = operationParsers.stream()
                                    .filter(ip -> ip.matches(input))
                                    .findAny()
                                    .map(parser -> parser.getOperation().calculate())
                                    .orElseThrow(() -> new IllegalArgumentException("no valid parser for input '"+input+"' found"));
    System.out.println("calculating" + result.getOperation())
}

With the change of the method signature (from The Calculator) we can reduce the number of responsibilities to two:

Result calculate(Expression expression) { 
    System.out.println(/* 1. print message */)
    return // 2. calculate the result
}

If you want to go further, we can even divide these responsibilities with the help of the Decorator Pattern:

class SimpleCalculator implements Calculator {
    Result calculate(Expression expression) { 
        return expression.calculate();
    }
}

class LogCalculationDecorator implements Calculator {
    private final Calculator calculator;

    Result calculate(Expression expression) { 
        System.out.println(/* ... */)
        return calculator.calculate(expression);
    }
}

// in main or else where
var calculator = new LogCalculationDecorator(new SimpleCalculator());
calculator.calculate(expression);

We can even add a Decorator to validate the expression; for example, to ensure that it is not a division with 0:

class ValidateCalculationDecorator implements Calculator {
    private final Calculator calculator;

    Result calculate(Expression expression) { 
        if(/* is division by zero*/) {
            throw new // exception;
        }
        
        return calculator.calculate(expression);
    }
}

// in main or else where
var calculator = new ValidateCalculationDecorator(
                 new LogCalculationDecorator(
                 new SimpleCalculator()));
calculator.calculate(expression);

It is not necessary to use the Decorator, I just want to mention that it would fit. Instead we can even simply use Dependency Injection:

class SimpleCalculator implements Calculator {
    private final Validator validator;
    private final Logger logger;

    Result calculate(Expression expression) { 
        validator.throwIfNotValid(expression)
        logger.log(expression);
        return expression.calculate();
    }
}

Answer 3

I want to add to the other answers and provide a different more drastic suggestion and a more generic and (I think) clean approach.

This is the perfect example where a nice class hierarchy shines.

This is also the perfect example where to use visitors.

I think in this case everything should be an expression.

First, the problem can be broken down in separate blocks:

• Representation of the expression in memory: The Expression class hierarchy
• Building the expressions from strings: The parser for infix notation
• Evaluating the expressions: evaluate() method on Expression interface (or ExpressionEvaluator visitor)
• Printing the expressions: toString() method on Expression interface (or ExpressionWriter visitor)

Get rid of MathResult and Operands. Everything can be an Expression.

interface Expression {
    Expression evaluate();
    String toString();
}

MathResult should be a Literal->Expression just like inputs, a Literal is just a number. You can also define different literals for different types if you want to be more generic (BooleanLiteral, Stringliteral, etc..)

class Int->Literal->Expression {
    int value;
    Expression evaluate() {return this};
    String toString() {
        return intToString(value);
    }
}

Operation is an expression, you will have to add classes for each operation:

abstract class BinaryExpression {
    Expression left;
    Expression right;
}

public class Addition->BinaryExpression {
     int value;
     Expression evaluate() {
          Expression r = right.evaluate();
          Expression l = left.evaluate()
          if (l and r are Int)
              return new Int( l.value() + r.value())
          else
              return new Addition(l, r);
    };

    String toString() {
        return left.toString() + " + " + right.toString();
    }
}

The Parser

The parser will turn strings into expression hierarchy, e.g:

"1 + 2" -> new Add(new Int(1), new Int(2))

A more complex parser might be able to read and create more complex expressions, e.g:

"1 * (2 + 4)" -> new Mul(Int(1),Add(Int(2),Int(4)))

There are software that helps you create parsers e.g.: javaCC

And then to use it:

Expression e = ExpressionInfixParser.parse("2 + 4");
print(e.toString()) //prints "2 + 4"
print(e.evaluate().toString()) //prints "6"

As you can see the class hierarchy is very generic and can represent many different expressions as long as you implement the operations you are interested in and the parser can read them. The interfaces are also very simple.

You can move the evaluate and toString code to separate visitors if you want to separate the representations from the logic. This can be useful, for example, if you want to be able to print your expression with different notations you can have different visitors that share a lot of code: ExpressionPrefixWriter, ExpressionInfixWriter, ExpressionPostfixWriter.

You can also easily extend the functionality with additional classes, e.g. you can have a Variable class that represent generic variables:

class Variable->Expression {
    String name;
    String toString() {
        return name;
    }
    Expression evaluate( Context context ) {
        if (context.contains(name))
            return context[name];
        else
            return this;
    }
}

And then evaluate expression in different contexts, a context can be as simple as:

class Context -> Map<String, Expression> {}

Example:

Expression e = ExpressionInfixParser.parse("a * ( 2 * 2 ) + b");

print(e.toString()) //prints "a * ( 2 * 2 ) + b"

print(e.evaluate().toString()) //prints "a * 4 + b"

Context c = {{"a", new Int(5)}}
print(e.evaluate(c).toString()) //prints "20 + b"

Context c = {{"a", Int(2)}, {"b", Int(9)}}
print(e.evaluate(c).toString()) //prints "17"

Sorry for the code syntax, it is more of a pseudocode.