1. Enum을 활용하고, 기본형 특화 인터페이스(IntBinaryOperator)를 활용하자.

private final Map<String, BinaryOperator<Integer>> functionByOperator = new HashMap<String, BinaryOperator<Integer>>() {{
		put("+", Integer::sum);
    put("-", (i, j) -> i - j);
    put("*", (i, j) -> i * j);
    put("/", (i, j) -> i / j);
}};

• 개선사항
  1. Enum을 활용해볼 수 있다.
  2. BinaryOperator보다는 기본형 특화 인터페이스인 IntBinaryOperator를 사용해도 좋다.

2. 접근제어자를 의미있게 사용

protected Integer calculate(String input) {...}

• protected를 쓴 의미가 있는가? 그때그때 맞는 접근제어자를 선택한다.

  

3. 메소드를 조금 더 분리하자

protected Integer calculate(String input) {
		isEmpty(input);
    String[] strings = input.split(DELIMETER);
		Integer total = 0;
    for (int i = 0; i < strings.length; i++) {
        String currentValue = strings[i];
        isValid(currentValue);
        if (isOperator(currentValue)) {
						total = functionByOperator.get(currentValue)
																			.apply(total, Integer.parseInt(strings[i + 1]));
            i++;
            continue;
        }
        total = Integer.parseInt(currentValue);
    }
    return total;
}

• 하나의 메소드에서 문자열 분리(연산자/피연산자)와 사칙연산 두 가지의 기능이 있다. 기능별로 더 작게 나누는게 좋다.

• 매번 isOperator() 메소드를 호출해 Operator인지 판단하는것은 효율적이지 않다. 내부적으로 continue를 활용해 for-loop의 동작을 제어하는것도 좋지 않은 패턴이다.

  사칙 연산은 피연산자(0) 연산자(1) 피연산자(2) 연산자(3) 피연산자(4) 의 패턴이 반복되니 홀수가 연산자임을 보장할 수 있으니 로직을 효율적으로 변경하자.

4. 상세한 주석은 권장되지 않는다.

: 메소드의 네이밍으로 의도를 나타내는것을 권장한다.

참고글

Clean Code - 주석(Comment)