Skip to content

[4기 - 한승원] 1~2주차 과제 : 계산기 구현 미션 제출합니다(사칙연산) #168

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 49 commits into
base: seungwon
Choose a base branch
from

Conversation

SW-H
Copy link

@SW-H SW-H commented Jun 11, 2023

📌 과제 설명

사칙연산에 대한 기능을 구현했습니다.
테스트코드나 추가적인 예외처리는 이후에 할 예정입니다.

👩‍💻 요구 사항과 구현 내용

계산기

✅ 피드백 반영사항

  • inputView/outputView(입출력)
    • inputView에 있던 출력 관련 메소드들 outputView로 이동(책임 구분)
    • 출력을 위한 메소드의 매개변수형을 String -> Double로 수정(계산 결과값의 형인 Double로 통일)
    • 사용자 입력과 입력값 검증 분리
  • Expression 클래스(수식)
    • 입력한 수식에 대한 검증 추가 (Expression::validate)
  • Operator(연산자)
    • 연산자의 우선순위를 비교하여 연산 순서를 결정하는 메소드(decideCalculationOrder)를 Calculator 클래스 -> Operator 클래스로 이동
  • Calculation 클래스(계산)
  • 계산시 반복적으로 사용되는 자료구조 상수화
  • 계산 로직 수정(Calculation 클래스)
    • 후위 연산식으로 변환없이 연산자의 우선순위에 따라 계산 후 중간 계산식 값을 반영
  • 불필요한 필드/메소드/구문 사용 삭제(Optional, 빈 생성자 등)

✅ PR 포인트 & 궁금한 점

+ 입출력에 대한 인터페이스를 두어 구현체에 대한 종속을 줄이고자 했고 연산자에 대한 enum 클래스를 통해 확장에 용이하도록 했습니다.
+ 그러나 객체지향적으로 구성한건지는 잘 모르겠어서 전반적인 구조에 대한 피드백 부탁드립니다.

  • 계산 로직(Calculation)에서 사용되는 메소드들의 네이밍과 최대한 작은 일만 하도록 나누려고 했는데 피드백 부탁드립니다.
  • 전반적으로 바뀐 부분이 많은데 수정 방향이 괜찮은지도 봐주셨으면 좋겠습니다😭

@SW-H SW-H requested a review from devcourse-ray June 11, 2023 21:47
@SW-H SW-H self-assigned this Jun 11, 2023
@wonu606 wonu606 requested a review from ICCHOI June 12, 2023 05:24
Copy link

@ICCHOI ICCHOI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

승원님 안녕하세요!! 일요일까지는 아니지만 그래도 월요일 오전에 끝내셨군요 :)
고생하셨습니다. 그런데 PR의 양이 많아서 조금 더 작은 기능 단위로 보내주시면 좋을 것 같아요.
객체지향 생활체조 원칙에 관한 이야기를 면담 때 드렸는데, 전체적으로 잘 지켜졌다고 보기 어려워요.
그래도 큰 구조는 나쁘지 않다고 생각이 듭니다! 조금만 더 추상화 해보시죠...!!!

Comment on lines 40 to 51
private void printCalculationGuide() {
System.out.println("1 + 2 * 3와 같은 형식으로 계산하고자 하는 식을 입력하세요.");
System.out.print("> ");
}

private void printMenuChoiceGuide() {
System.out.println("\n[다음 중 원하시는 항목을 숫자로 입력하세요]");
System.out.println("1. 조회");
System.out.println("2. 계산");
System.out.println("3. 종료");
System.out.print("> 선택 : ");
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

inputView에서 Output을 진행하고 있네요!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

outputView클래스로 이동하여 수정하겠습니다

Comment on lines 53 to 57
private void validateUserMenuChoice(String userInput) throws IllegalArgumentException {
if (!USER_MENU.contains(userInput)) {
throw new IllegalArgumentException(ExceptionMessage.INVALID_INPUT);
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

contains 메소드를 사용하는 것도 좋지만, enum을 좀 더 잘 활용하려면 Menu를 검증하는 메소드를 직접 만드는 것도 좋아보여요.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

enum클래스 내에 메소드를 만들어서 사용하라는 말씀이실까요? enum을 더 잘 활용하려면 이라는 말이 무슨 뜻인지 잘 모르겠어요..

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

넵! 아래의 코멘트 참고하시면 될 것 같습니다

Comment on lines 13 to 16
private static final Set<String> USER_MENU =
Arrays.stream(UserMenu.values())
.map(UserMenu::getValue)
.collect(Collectors.toSet());
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UserMenu Enum이 있음에도 불구하고 Set을 사용하신 이유가 궁금해요.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

InputView에서 UserMenu의 유효성을 검사하고 있는 부분이 좀 어색하게 보였어요.
해당 기능은 UserMenu가 담당해도 되지 않을까요?

Comment on lines 15 to 17
private static final Map<String, UserMenu> values =
Collections.unmodifiableMap(Stream.of(values())
.collect(Collectors.toMap(UserMenu::getValue, Function.identity())));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

values라는 이름은 해당 자료구조가 어떤 역할을 하는지 알아보기 어려워요.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

위와 같은 이유로 삭제했습니다

Comment on lines 24 to 26
public static UserMenu get(String input) {
return Optional.ofNullable(values.get(input)).orElse(TERMINATE);
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

어떤 의도로 Optional을 사용하신 걸까요?
메소드 체이닝이 너무 많아 코드 가독성을 해치는 걸로 보여요

case CALCULATE:
String expression = inputView.inputExpression();
Integer output = calculate(expression);
outputView.print(String.valueOf(output));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

valueOf 함수가 불필요하게 너무 반복되는 거 같아요.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

outputView를 수정하도록 하겠습니다

Comment on lines 16 to 17
SUBTRACTION("-", 2, (operand1, operand2) -> Integer.valueOf(
operand2 - operand1
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

인자값이 모두 Integer인데 Integer.valueOf를 사용하는 이유가 있을까요?

Comment on lines 48 to 51
public static Operator getSymbol(String operator) {
return Optional.ofNullable(operators.get(operator))
.orElseThrow(() -> new IllegalArgumentException(ExceptionMessage.INVALID_SYMBOL));
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public static Operator getSymbol(String operator) {
return Optional.ofNullable(operators.get(operator))
.orElseThrow(() -> new IllegalArgumentException(ExceptionMessage.INVALID_SYMBOL));
}
public static Operator getOperator(String input) {
return Arrays.stream(Operator.values())
.filter(operator -> operator.symbol.equals(input))
.findFirst()
.orElseThrow(() -> new IllegalArgumentException(ExceptionMessage.INVALID_SYMBOL));
}

이런 방법은 어떠신가요?

Comment on lines 31 to 33
private static final Map<String, Operator> operators =
Collections.unmodifiableMap(Stream.of(values())
.collect(Collectors.toMap(Operator::getSymbol, Function.identity())));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

해당 Map의 쓰임새가 좀 더 명시적으로 표현 될 수 있으면 좋을 것 같아요.

Comment on lines 28 to 29
System.out.println(illegalArgumentException.getMessage());
return UserMenu.TERMINATE.getValue();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UserMenu는 사용자가 입력값으로 기대할 수 있는 선택지 중 하나라고 생각해요.
즉, 기능 선택이라는 역할을 하고 있는거죠.
그런데 예외에 대한 처리로 인위적으로 TERMINATE를 던지면 예외처리와 기능 선택이라는 작업이 섞이게 되요.
예외처리는 예외처리를 위한 다른 로직이 필요하다고 생각해요.

@SW-H
Copy link
Author

SW-H commented Jun 12, 2023

승원님 안녕하세요!! 일요일까지는 아니지만 그래도 월요일 오전에 끝내셨군요 :)
고생하셨습니다. 그런데 PR의 양이 많아서 조금 더 작은 기능 단위로 보내주시면 좋을 것 같아요.
객체지향 생활체조 원칙에 관한 이야기를 면담 때 드렸는데, 전체적으로 잘 지켜졌다고 보기 어려워요.
그래도 큰 구조는 나쁘지 않다고 생각이 듭니다! 조금만 더 추상화 해보시죠...!!!

빠른 리뷰 감사합니다. 코드를 짜다보니 쪼갤 타이밍을 놓쳐 PR양이 많아졌네요. 다음 PR부터는 좀 더 작은 범위로 보내고 추상화를 더 생각하면서 수정하겠습니다

Copy link

@devcourse-ray devcourse-ray left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

과제하시느라 고생 많으셨어요~
객체지향적인 구조를 많이 고민하신게 보이네요
한 가지 팁을 드리자면 너무 많은걸 한 번에 고려하기보단 차근차근 단계를 밟아가면 훨씬 더 전체적인 그림을 빠르게 완성 할 수 있을 것 같아요!

Calculator calculator = new Calculator(inputView, outputView);
calculator.run();
}
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

마지막 라인이 빠져있네요

}

private Integer calculate(String input) {
char[] expression = input.toCharArray();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

입력 값이 아무 이상없이 들어올거라고 단정짓기에는 사용자의 입력이 너무 자유롭지 않을까요>

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

validation 관련해 추가하겠습니다

Comment on lines 41 to 63
for (int i = 0; i < expression.length; i++) {
if (Character.isWhitespace(expression[i])) {
continue;
}
if (Character.isDigit(expression[i])) {
StringBuffer operand = new StringBuffer();
while (i < expression.length && Character.isDigit(expression[i])) {
operand.append(expression[i++]);
}
i--;
Operands.push(Integer.parseInt(operand.toString()));
} else if (expression[i] == '(') {
Operators.push(expression[i]);
} else if (expression[i] == ')') {
evaluateOperators(Operands, Operators);
} else {
while (!Operators.empty() && comparePriority(expression[i], Operators.peek())) {
Operands.push(Operator.calculate(
String.valueOf(Operators.pop()), Operands.pop(), Operands.pop()
));
}
Operators.push(expression[i]);
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

기능에 비해 로직이 너무 복잡한 것 같아요
너무 모든걸 한 번에 고려하기보단 하나씩 차근차근 접근해보는건 어떨까요?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

넵 계산 관련 로직도 수정하고 그에 따라 메소드도 분리해보았습니다(Calculation 클래스 확인 부탁드려요).

Comment on lines 49 to 50
return Optional.ofNullable(operators.get(operator))
.orElseThrow(() -> new IllegalArgumentException(ExceptionMessage.INVALID_SYMBOL));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기서 굳이 optional 로 한 번 감싸줘야할까요?
optional을 효율적으로 쓰는 방법은 어떤게 있을까요?

Copy link
Author

@SW-H SW-H Jun 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

operators.get()함수의 반환값이 null이 될 수 있어서(입력한 operator 가 정의된 +,-,*,/에 해당되지 않는 다면) exception처리를 하기 위해 위처럼 작성 했습니다. 다만 Optional로도 감싸주고 exception도 날리는건 중복으로 처리하는 것 같아서 둘중에 하나로만 했어도 될 것 같습니다. 아래는 optional에 대해 제가 찾아보고 이해한 내용입니다.

Optional은 호출 하는 쪽에 '반환 값이 null일 수도 있다(반환할 결과값이 없다)' 라고 명시적으로 알려주는 역할을 가진 것으로 이해했습니다. 그러니까 메서드가 반환한 결과 값이 '없음'을 표현하고자 하는데 null을 반환하는 것은 에러를 유발하거나 해서는 안되는 행동으로 여겨지니 도입된 개념입니다. 호출하는 쪽에서 null 체크를 쉽게 할 수 있어 NullPointerException을 방지할 수 있습니다.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기는 잘 고민해보고 더 궁금한 내용이 있으면 언제든 말씀해주세요~

Comment on lines 3 to 11
public class ExceptionMessage {

public static final String DIVIDED_BY_ZERO = "0으로 나눌 수 없습니다.";
public static final String INVALID_SYMBOL = "잘못된 연산 기호입니다.";
public static final String INVALID_INPUT = "잘못된 입력입니다";

private ExceptionMessage() {
}
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

메시지 관리를 하는 방법에는 어떤것들이 있을까요?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

enum을 사용해 에러코드와 함께 관리하거나 Exception 클래스를 상속받은 클래스를 구현할 수도 있을 것 같습니다. 다만 저는 개수가 그렇게 많지 않고 단순히 에러 메시지를 코드화하여 관리하기 위함이라 위와 같이 사용했습니다.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

개발하면서 그 어떤것도 확답을 내릴 수 없어요
단순한 계산기 하나에도 발생할 수 있는 에러는 굉장히 많고 그에 맞는 에러메시지도 다양하겠죠?

갑자기 변경 할 때 전체적인 구조가 흔들리는 것 보단 지금은 괜찮더라도 추후 변경을 고려하여 설계하는 연습도 필요해요

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

추후 변경을 고려하여 설계하라는 점은 염두에 두겠습니다🤩

그런데 예외메시지만 관리한다는 관점에서 볼때 enum을 사용하는 것과 현재방법의 차이를 잘 모르겠습니다,, 그리고 Exception 클래스를 상속받은 사용자 정의 예외클래스를 구현하는 것은 현재 예외의 개수도 적고 표준예외로 충분히 표현 가능하다고 느껴지는데 과한 구현이지 않을까 싶어서 필요하다면 나중에 확장하면서 고려하면 되지않을까 싶은데... 어떨까요....?🥹

제가 놓치고 있는 부분이 있을까요?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. 표준 예외만으로 충분히 표현 가능하다면 커스텀한 에러 메시지를 따로 관리 해야할까요?
  2. 구조가 달라지는걸 확장이라고 표현할 수 있을까요?
  3. 에러메시지를 관리하는게 enum 밖에 없을까요?
  4. static 을 어떻게 써야 효율적일까요?

제가 이 클래스를 작성했다고 생각해보면 당장 위 고민들이 떠오르네요
더 많은 고민거리들이 있겠지만 최소한 이정도는 스스로 답변을 내릴 수 있도록 고민해봐주세요

Comment on lines 18 to 19
public CalculatorInputView() {
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

빈생성자는 의도하신걸까요?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

빠뜨리고 수정한 것 같습니다. 삭제하겠습니다

mavenCentral()
java {
toolchain {
languageVersion = JavaLanguageVersion.of(11)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Comment on lines 52 to 55
} else if (expression[i] == '(') {
Operators.push(expression[i]);
} else if (expression[i] == ')') {
evaluateOperators(Operands, Operators);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bracket은 후순위로 진행해주시면 될 것 같아요

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

우선은 삭제했습니다

@SW-H SW-H requested review from devcourse-ray and ICCHOI June 15, 2023 11:49
@SW-H SW-H marked this pull request as draft June 15, 2023 12:53
@SW-H SW-H marked this pull request as ready for review June 15, 2023 13:25
Copy link

@ICCHOI ICCHOI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

승원님 고생하셨습니다!
많이 성장하신게 보이네요!! 💯
Calculation와 관련된 로직이 조금 복잡하고 이해하기가 어려워서요..
해당 부분을 개선한 후 테스트 코드를 작성하시면 될 것 같아요 :)

public static UserMenu get(Integer input) {
return Arrays.stream(values())
.filter(menuNum -> menuNum.menu.equals(input))
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

메소드 레퍼런스를 사용해보시는 건 어떨까요?

INQUIRY("1"),
CALCULATE("2"),
TERMINATE("3");
INQUIRY(1), CALCULATE(2), TERMINATE(3), ERROR(-9999);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TERMINATE와 ERROR를 분리하셨네요 👍

Comment on lines +42 to +46
Collections.sort(operators, (operator1, operator2) ->
Integer.compare(getSymbol(operator1).getPriority(), getSymbol(operator2).getPriority())
);
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public static void decideCalculationOrder(List<String> operators) {
Collections.sort(operators, (operator1, operator2) ->
Integer.compare(getSymbol(operator1).getPriority(), getSymbol(operator2).getPriority())
);
}
public static void decideCalculationOrder(List<String> operators) {
operators.sort(Comparator.comparingInt(Operator::getPriority));
}

메서드 레퍼런스를 사용하면 좀 더 가독성 있게 읽혀요

Comment on lines +44 to +45
Double operand1 = Double.parseDouble(parsedExpression.get(operatorIdx - 1));
Double operand2 = Double.parseDouble(parsedExpression.get(operatorIdx + 1));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parsedExpression.get(operatorIdx - 1)은 따로 선언해 준 후 넘겨주면 좀 더 가독성 좋게 읽힐 것 같아요

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

int operatorIdx = parsedExpression.indexOf(operator);
String operand1 = parsedExpression.get(operatorIdx - 1);
String operand2 = parsedExpression.get(operatorIdx + 1);
return new Double[] {Double.parseDouble(operand1), Double.parseDouble(operand2)};

이렇게 수정하는 방향은 어떨까요?

Comment on lines 33 to 35
for (int cnt = 0; cnt < count; cnt++) {
parsedExpression.remove(operatorPosition);
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cnt와 count가 헷갈려 보여서, i로 사용하는 편이 좋을 것 같아요

Comment on lines +51 to +60
try {
CalculatorOutputView.printCalculationGuide();
Expression expression = inputView.inputExpression();
Calculation calculator = new Calculation(expression);
Double output = calculator.calculate();
outputView.printCalculationResult(output);
repository.save(expression.getExpression(), output);
} catch (ArithmeticException arithmeticException) {
System.out.println(arithmeticException.getMessage());
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ArithmeticException이 일어 날 수 있는 곳만 try-catch로 잡는 편이 좋지 않을까요?

Comment on lines +26 to +30
private boolean validate() {

Matcher matcher = EXPRESSION_FORMAT.matcher(expression);
return matcher.matches();
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
private boolean validate() {
Matcher matcher = EXPRESSION_FORMAT.matcher(expression);
return matcher.matches();
}
private boolean isNotValidate() {
Matcher matcher = EXPRESSION_FORMAT.matcher(expression);
return !matcher.matches();
}

!를 쓰는 대신 메서드에 부정을 담으면 좀 더 명시적으로 읽힐 수 있어요!

Copy link
Author

@SW-H SW-H Jun 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

깃헙에 업로드 하면서 오류가 있었나봐요... 아래처럼 작성하였습니다! 혹시 !를 쓸일이 있으면 알려주신대로 부정문을 사용하겠습니다!

private boolean validate() {
  Matcher matcher = EXPRESSION_FORMAT.matcher(expression);
  return matcher.matches();
}

import java.util.Map;

public class CalculatorRepository implements Repository {
private static final Map<String, Double> storage = new LinkedHashMap<>();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이런 구조면 LinkedHashMap이 아니게 될 때는 저장 이력을 순서대로 저장하지 못하겠군요?

parsedExpression.add(operatorPosition - 1, String.valueOf(calculationRes));
}

private Double[] extractOperands(String operator) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extractOperands는 어떤 기능을 하는 메서드 인가요?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

매개변수로 주어진 Operator에 해당하는 피연산자를 뽑아내는 메서드입니다!

Comment on lines +32 to +35
private void removeCompletedExpression(int operatorPosition, int count) {
for (int cnt = 0; cnt <= count; cnt++) {
parsedExpression.remove(operatorPosition);
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cnt와 count가 동일한 의미를 가지고 있어서 이름을 바꾸는 편이 좋아보여요

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

넵 그 부분 수정하면서
두번째 인자도 삭제할 수식의 길이를 의미함을 알아보기 좋도록 length로 변경했습니다

private void removeCompletedExpression(int operatorPosition, int length) {
  for (int i = 0; i <= length; i++) {
	  parsedExpression.remove(operatorPosition);
  }
}

Comment on lines +19 to +20
parsedExpression = expression.split();
operators = extractOperators();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이렇게만 사용할거라면 굳이 미리 클래스에 선언해야할까요??

Comment on lines +22 to +28
for (String operator : operators) {
int operatorPosition = parsedExpression.indexOf(operator);
Double[] operands = extractOperands(operator);
Double calculationRes = Operator.calculate(operator, operands[0], operands[1]);
storeIntermediateResult(operatorPosition, calculationRes);
removeCompletedExpression(operatorPosition, operands.length);
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

더 단순하게 정리해볼까요?

}

private Double calcFinalResult() {
return Double.parseDouble(parsedExpression.get(0));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

parsedExpression의 상태에 따라서 해당 메소드가 결과가 다르고 심지어 에러도 발생할 수 있겠네요

Comment on lines +31 to +33
public static Double calculate(String operator, Double operand1, Double operand2) {
return getSymbol(operator).operation.apply(operand1, operand2);
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

operator를 받아와야하나요?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

String으로 받은 연산자에 해당하는 함수를 실행하기 위해 위에처럼 사용했습니다..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants