본문 바로가기

우아한테크코스/코드 리뷰

문자열 계산기

1주 차 대망의 첫 번째 미션은 문자열 계산기였다.

크루들과 친해지기 위해 했던 연극과 미션이 겹쳐서 미션에 많은 시간을 투자하지 못한 아쉬움이 있었다.

포비는 첫 주차부터 너무 잘하려고 하지말라고 하셨지만 그래도 첫 주차다보니 욕심이 나는 건 어쩔 수 없었다.


단위 테스트를 실습해보는 미션이었다.
우테코에서 처음으로 받는 미션이었고 처음으로 해보는 페어 프로그래밍이었다.

첫 미션이기 때문에 잘하고싶은 열정, 욕심만 가득했던 한 주였던 것 같다.

리뷰어님께 받은 리뷰내용과 연극 팀원들끼리 진행한 코드리뷰를 정리하고자 한다.


나누기를 할 때 0으로 나누는 것을 검사하고, 0으로 나눈다면 오류를 throw하는 내용인데 모호한 파라미터 명과 모호한 숫자로 인해 if 조건식이 무엇을 판별하는지 명시적으로 보이지 않았다.

private static void validDivideZero(double secondOperand) {
    if (secondOperand == ZERO) {
        throw new ArithmeticException(CAN_NOT_DIVIDE_ZERO);
    }
}

나누기할 때 0으로 나누는지 검사하는 메소드를 따로 분리해서 명확한 파라미터와 비교하는 값을 상수로 명확하게 표현, if 조건식이 어떤 조건을 판별하는지 명시적으로 보이고자 노력했다.


기존에는 처음 알게된 함수형 인터페이스 BiFuction에 계산식을 넣고 Map에서 +, - 등 연산자로 get을 했을 때 계산식이 리턴되는 방식으로 구현했었다.

첨부해주신 링크와 enum에 대해 학습하고 enum으로 구현하면서 enum을 사용하는게 더 좋다는 생각이 들었다.

맵으로 할때는 아래와 같이 표현되던게

operators.put("+", (num1, num2) -> num1 + num2);
operators.put("-", (num1, num2) -> num1 - num2);
operators.put("*", (num1, num2) -> num1 * num2);
operators.put("/", (num1, num2) -> num1 / num2);

enum으로 할 때는 무슨역할을 하는지 더 명확하게 표현되는 걸 볼 수 있다.

PLUS("+", (num1, num2) -> num1 + num2),
MINUS("-", (num1, num2) -> num1 - num2),
MULTIPLICATION("*", (num1, num2) -> num1 * num2),
DIVISION("/", (num1, num2) -> num1 / num2);

이번 미션에서는 코드간의 관계가 명확히 표현되므로 enum을 사용하는게 더 좋다고 느꼈다.

public enum Calculation {
    PLUS("+", (num1, num2) -> num1 + num2),
    MINUS("-", (num1, num2) -> num1 - num2),
    MULTIPLICATION("*", (num1, num2) -> num1 * num2),
    DIVISION("/", (num1, num2) -> {
        validDivideZero(num2);
        return num1 / num2;
    })

0인지 검사하는 부분까지 포함하여 enum으로 작성하였다.


계산을 진행하는 부분의 내용이였는데 메소드 호출시의 파라미터가 너무 복잡하기 때문에 확실히 어떤 파라미터가 넘어가는지 보기 힘들어보였다.

stackUpValue = calculation.calculate(stackUpValue, operands.poll());

전체적으로 코드를 다듬으면서 파라미터를 두개로 줄였고 파라미터로 뭐가 넘어가는지 명확하게 보이도록 수정하였다.


연극 팀인 '덜 우아한 형제들'의 팀원들과 코드리뷰를 하면서도 많은 것을 배울 수 있었다.

for (int index = 1; index < formulas.size(); index += OPERATOR_INDEX_VALUE) {
   operators.add(formulas.get(index));
}

index < formulas.size(); 처럼 for문의 조건을 지정해두면 for문이 반복될때마다 formulas.size()메소드가 호출되면서 미미하지만 효율성이 저하될 수 있기 때문에 다음과 같이 수정하는게 좋다. 이렇게 하면 한 줄이 늘어나기 때문에 반복이 적은 일반적인 경우엔 가독성을 고려해서 size()메서드로 구현하는게 더 나을 수도 있는 것 같다.

int formulasSize = formulas.size();
for (int index = 1; index < formulasSize; index += OPERATOR_INDEX_VALUE) {
    operators.add(formulas.get(index));
}

public static String input() {
    System.out.print("계산식을 입력하세요 : ");
    return new Scanner(System.in).nextLine().trim();
}

메소드 호출을 할 때마다 Scanner객체를 생성하게 되니까 Scanne를 굳이 매번 생성할 필요가 없었다. Scanner를 static 멤버 변수로 선언하면 버려지는 객체가 없기에 메모리를 효율적으로 쓸 수 있다.


private static int calculateByInput() {
    try {
        Formula formula = new Formula(InputView.input());
        Formulas formulas = new Formulas(formula);
        return Calculator.calculateByFormulas(formulas);
    } catch (IllegalArgumentException e) {
        OutputView.exceptionMessage(e.getMessage());
        return calculateByInput();
    }
}

재귀 형태로 프로그램을 진행하는 것 보다 반복문으로 진행하는 것이 더 효율적이다. 재귀 형태로 진행하면 우선 stack overflow의 가능성이 생긴다. 또한 메서드 호출 비용과 메서드를 열고 닫을 때 마다 각각의 스택 프레임을 생성하고 소멸시키는 비용이 들기 때문이다.


경력자 리뷰어분의 코드 리뷰를 통해 클린 코드를 향해 한걸음 나아간 기분이고, enum등 새로운 내용들도 학습 할 수 있었다.

또한 팀원들과 각자 리뷰받은 내용을 공유하고 서로의 코드를 지적하며 뭐가 더 좋은 코드에 가까운지 토론하면서 많은 것들을 배우고 느낄 수 있었다.

하지만 실습명이 단위 테스트 실습인데 처음 공부해보는 테스트 코드에 대한 공부는 많이 하지 못했다.

다음 주 부터는 테스트 코드에 더 신경을 쓰고 Junit API에 익숙해지도록 열심히 공부해야 겠다.

그리고 이번주에 미션을 진행하면서 학습한 enum과 함수형 인터페이스에 대해서도

다시 한 번 정리하면서 학습할 것이다.

'우아한테크코스 > 코드 리뷰' 카테고리의 다른 글

블랙잭 2  (0) 2020.03.30
블랙잭 1  (0) 2020.03.18
로또 게임 2  (0) 2020.03.02
로또 게임 1  (0) 2020.02.26
자동차 경주 게임  (0) 2020.02.18