[4기 -유원우] 1~2주차 과제 : 계산기 구현 미션 제출합니다. #167

wonu606 · 2023-06-11T18:01:47Z

📌 과제 설명

✅ 1을 입력 받으면 계산을 수행합니다.
✅ 계산은 중위연산식만을 수행하며 결과는 int값으로 출력합니다.(소수가 나올 시, 반올림하여 출력)

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

feat: 조회와 계산 기능을 담을 인터페이스 구현

조회와 계산 기능에 사용할 인터페이스인 Strategy를 만들었습니다.
Strategy와 연관관계인 Persistence 인터페이스를 만들었습니다.
Persistence 인터페이스는 저장소의 인터페이스로 사용할 예정입니다.

test: 저장소 구현체 테스트 작성

저장소에 저장할 형태의 데이터 객체 구현
저장소 테스트 코드 작성
저장소는 연산식과 값을 보관합니다.
순번으로도 검색할 수 있습니다. 최소1 - 최대 사이즈

feat: 저장소 구현체, 메시지를 담을 클래스 구현

저장소 구현체를 구현하였습니다.
테스트 코드 통과
Message 클래스를 만들어 문자열을 직접 타이핑하여 발생하는 실수를 줄이려 하였습니다.

feat: Converter 인터페이스와 StringToIntegerConverter 객체 구현

CalculatorApp에서 Integer.parseOf가 아닌 컨버터를 통해 변환합니다.
(잘못된 입력으로 오류가 발생할 가능성이 있기 때문에 수정하였습니다.)

refactor: CalculatorApp 객체에서 Input, Print 필드 제거 및 함수화

Input과 Output이 필드에 있어야 할 필요성이 없다고 생각하여 제거하였습니다.
execute 함수에 코드를 함수화하였습니다.
strategies.get()시 발생할 수 있는 예외를 처리하였습니다.

refactor: 입력시 throw IOException 예외 명시 삭제

메소드에서 바깥으로 계속 던져주는 문제 발생으로 인한 수정
IOException을 처음 넘겨주는 객체가 불필요한 예외를 넘겨준다 생각하여 제거하였습니다.

feat: 연산자 구현

연산자를 enum으로 구현하였습니다.

feat: 중위 연산식 유효 검사 클래스 구현

WhiteSpace 단위로 입력된 중위 연산식이 유효한 지를 검사하는 클래스를 구현하였습니다.

feat: 중위 연산식 유효 검사 테스트

정상적인 후위 연산식으로 들어올 경우 false
정상 중위 연산식일 경우 true를 뱉도록 하였습니다.

feat: 중위 연산식을 후위 연산식으로 바꾸는 converter 구현
test: 중위 연산식을 후위 연산식으로 변환하는 Converter 테스트 작성 및 통과
feat: 후위 연산식을 계산하는 클래스 구현
test: 후위 연산식을 계산하는 클래스 테스트 작성 및 통과
chore: 불필요한 import문 제거 및 디렉토리 재정렬

import 정렬
테스트 코드 디렉토리가 source와 다르게 배치되어 자동 테스트 생성시 발생하는 문제 떄문에
테스트 코드 재배치

test: 중위 연산식을 입력 받아 계산하는 클래스 테스트 작성

중위 연산식 테스트 작성
Overflow 메시지 추가

feat: 계산하는 구현체 구현 및 테스트 통과

계산 기능 추가

✅ 피드백 반영사항

✅ StringToInteger를 클래스로 나눌 만큼 역할을 수행하지 않고, Validator가 해야할 역할을 수행하여 제거
명시적이지 않은 이름 Strategy -> CalculrateStrategy로 변경하였습니다. 하지만 이부분은 아직 명시적이지 않다고 생각하여
문제가 있다면 재수정하겠습니다!!
✅ 테스트 코드에서 예외 테스트 코드에 assertThatcode() -> assertThrowBy로 변경하였습니다.
✅ 테스트 코드를 given-when-then 방식을 적용하였습니다.
전략 패턴을 가져오는 곳에서 get() 메서드이지만, 에러를 뱉어 getOrThrow로 함수명을 변경하였습니다.
이부분 또한 문제 있을 시 변경하겠습니다!!

✅ PR 포인트 & 궁금한 점

계산과 조회 기능을 담당할 Strategy에서 input, output, store를 주입받습니다.
하지만 조회 기능에서는 입력을 사용하지 않으니 주입받으면 안되는 거 아닌가 하는 생각이 듭니다.
한 편으로는, 추가 기능이 들어왔을 때의 확장성을 생각하여 주입 받는 것이 맞는거 아닌가 하는 생각 또한 듭니다.
(예를 들어, 2번쨰 입력을 보여주는 기능 추가)

ICCHOI

안녕하세요 원우님! 코멘트 남겼으니 확인 부탁드려요 :)
Strategy와 관련된 로직들이 많은데, Strategy를 구현하지 않으셔서 리뷰에 어려움이 있는 거 같아요

입출력 다음 어떤 구현을 먼저 했어야 다른 사람들이 코드를 이해하기 좋을지 궁금합니다. Strategy 인터페이스부터 만들었는데, 이 코드를 어디에 쓰려는 건지 나만 알 수도 있겠다는 생각이 들었습니다.
- 최소 단위의 기능을 만족하면 구현 순서는 중요하지 않다고 생각해요! 지금은 Strategy가 많은 로직에 영향을 끼치고 있어서 그렇게 느끼시는게 당연하다고 생각됩니다.
Message 클래스를 enum이 아닌 abstract class로 만들었습니다.
일반적으로 enum으로 만드는 거 같은데 전 getString()을 불러오는 것이 가독성을 헤친다고 생각하였습니다. 이렇게 써도 될까요?
- Message 클래스를 추상 클래스로 만들면 어떤 장점이 있나요?
- getString()을 불러오는 것이 어떤 면에서 가독성을 헤친다고 생각하셨나요?
- Message 클래스를 일반적으로 enum으로 만든다고 하셨는데 이유가 뭘까요?
점점 코드가 커지다 보니 연관 관계인 클래스들이 많아졌습니다.
커밋 단위로 나누기에도 애매하고 커밋 메시지에 구현 여부를 굳이 써야 하나라는 생각이 들었습니다.
제 코드에서는 CalculationResult 객체입니다. 일반적인 예시는 Dto 역할만을 수행하는 클래스입니다.
- 커밋 단위는 작업하기 편한 방식으로 진행 해주셔도 될 것 같습니다.

ICCHOI · 2023-06-11T18:39:42Z

calculator/src/main/java/com/wonu606/calculator/model/CalculationResult.java

+    @Override
+    public boolean equals(Object o) {
+        if (this == o) {
+            return true;
+        }
+        if (o == null || getClass() != o.getClass()) {
+            return false;
+        }
+        CalculationResult that = (CalculationResult) o;
+        return Double.compare(that.getResult(), getResult()) == 0 && Objects.equals(
+                getExpression(), that.getExpression());
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(getExpression(), getResult());
+    }


equals와 hashCode를 재구현하신 이유가 뭘까요?

재구현 이유

Object에서 상속받은 equlas와 hashCode로는
CalculationResult를 비교할 수 없다고 생각하여 재구현하였습니다.

사용

저장소에 CalculationResult이 제대로 저장되었는지 확인할 때 사용하였습니다.

기본 equals와 hashCode 는 어떻게 구현되어있을까요?

Object.equals

'=='와 같은 연산을 비교하여 기본 equals는 참조값 비교입니다.

Object.hashCode

해시코드가 native로 되어있어 정확한 코드를 보진 못했습니다.
하지만 공식 문서에서 두 객체의 equals에 따라 동일한 경우, 같은 hasocde가 나와야 한다고 되어있습니다.
내부적으로 해시함수에 따른 해시를 생성하고, 이후 equals를 통해 한번 더 비교하는 것이 아닌가 추측이 됩니다.

저는 자동 완성으로 equals와 hash를 만들어 기대 성능인 O(1)이 안나올 수도 있다고 생각하게 되었습니다.

저장소에 CalculationResult이 제대로 저장되었는지 확인할 때 사용했다는 말은, 테스트 코드를 위해 비즈니스 로직을 변경한 것으로 보이는데 이게 올바른 방법일까요?

일반적으로 테스트 코드를 위해 비즈니스 로직을 변경한 것이 문제가 된다고 생각합니다.
하지만 CalculationResult는 값만을 가지고 있고, 값만을 가지고 있는 객체는 Value Object라고 하여
동등성이 보장되어야 한다고 생각하였습니다.

궁금한 점이 있습니다!! 인창 멘토님
VO 객체라도 equals와 hashcode를 사용 없이 구현하는 예외로 두면 안되는 건가요??
제가 그렇게 생각한 이유는 VO 객체에 두 메소드를 구현하지 않으면
추후 Set<CalculationResult>를 사용하고
CalculationResult에 equals와 hashcode이 구현되지 못한 것을 보고 사용할 경우
실수를 유발할 수 있다고 생각하기 때문입니다.

ICCHOI · 2023-06-11T23:33:09Z

calculator/src/main/java/com/wonu606/calculator/converter/StringToIntegerConverter.java

+public class StringToIntegerConverter implements Converter<String, Integer> {
+
+    @Override
+    public Integer convert(String input) {
+        Integer result = null;
+        try {
+            result = Integer.parseInt(input);
+        } catch (NumberFormatException exception) {
+            throw new IllegalArgumentException(Message.INVALID_INPUT);
+        }
+        return result;
+    }
+}


String을 Integer로 바꾸기 위한 작업인데, 조금 과하다는 느낌이 드는 거 같아요.

저도 과하는 생각을 하였습니다. 또한 Converter 안에서 예외를 뱉는 것도 Converter의 역할에서 벗어난다고 생각이 들었습니다.

변경

integer로 변환하지 않고 String 그대로 처리하는 것으로 변경하였습니다.

StringToIntegerConverter 클래스를 삭제하였습니다.

String selection = input.getInput();

ICCHOI · 2023-06-11T23:34:37Z

calculator/src/main/java/com/wonu606/calculator/converter/StringToIntegerConverter.java

+        try {
+            result = Integer.parseInt(input);
+        } catch (NumberFormatException exception) {
+            throw new IllegalArgumentException(Message.INVALID_INPUT);


특정 타입을 다른 타입으로 바꾸는 작업을 수행하는 Converter가 잘못된 입력에 관한 예외를 반환하는 건 조금 어색해보여요.
이 책임은 Input에서 담당하거나 Validate를 위한 객체에서 담당하는게 맞지 않을까요?

Validate로 외부에서 담당하는 것이 맞다고 생각합니다.
단순히 String을 Integer로 변경하는데 두 객체를 생성하는 것이 맞을까 생각하여
StringToIntegerConverter을 삭제하였습니다.

ICCHOI · 2023-06-11T23:38:05Z

calculator/src/main/java/com/wonu606/calculator/storage/ResultStore.java

+    @Override
+    public List<CalculationResult> findAllResult() {
+        return new ArrayList<>(store);
+    }


새로운 인스턴스에 반환하는 이유가 있을까요?

그대로 반환해주게 된다면 외부에서 store 값을 수정할 수도 있겠다고 생각했습니다.
하지만 제가 원하는 동작이 아니여서 새로운 인스턴스로 반환하였습니다.
CalculationResult는 필드가 final이라 따로 객체를 생성하지는 않았습니다!

List를 일급 컬렉션으로 만들어 반환하는 것도 고려해보시죠

ICCHOI · 2023-06-11T23:39:02Z

calculator/src/main/java/com/wonu606/calculator/strategy/Strategy.java

+public interface Strategy {
+
+    void execute(Input input, Print printer, Persistence store);
+}


어떠한 전략인지 파악할 수 있게 좀 더 명시적인 인터페이스 이름이 필요할 것 같아요.

계산기에서만 쓰는 전략이므로 CalculatorStrategy로 변경하였습니다.

ICCHOI · 2023-06-11T23:40:16Z

calculator/src/test/java/com/wonu606/converter/StringIntegerConverterTest.java

+    void testOverflow() {
+        String input = "2147483648";
+
+        assertThatCode(() -> converter.convert(input))


assertThatCode와 assertThatThrownBy의 차이점이 무엇인가요?

assertThatCode와 assertThatThrownBy의 차이를 모르고 썼었습니다.

찾아보니
assertThatCode:

return AssertionsForClassTypes.assertThatCode(shouldRaiseOrNotThrowable);

assertThatThrownBy:

return assertThat(catchThrowable(shouldRaiseThrowable)).hasBeenThrown();

예외가 발생해야 하는 경우 assertThatThrownBy
예외가 발생하지 않아야 하는 경우 assertThatCode로 정리할 수 있었습니다.

차이를 모르고 혼용하여 사용하였습니다.

변경

StringToIntegerConverter 객체를 삭제하여 테스트 코드도 삭제하였습니다.

ICCHOI · 2023-06-11T23:40:59Z

calculator/src/test/java/com/wonu606/storage/ResultStoreTest.java

+        assertThat(store.findAllResult().size()).isEqualTo(0);
+
+        CalculationResult calculationResult = new CalculationResult("2 + 2", 4.0d);
+        store.saveResult(calculationResult);
+
+        assertThat(store.findAllResult().size()).isEqualTo(1);


given, when, then 패턴을 따라서 테스트 코드를 작성해주세요

given - when - then에 대해 명확히 알지 못해 찾아보았습니다.
given: 초기 상태, when: 주어진 조건, then: 그 떄의 상태 라고 이해하였습니다.
제가 이해한 것이 맞을까요?

변경

이해한 given - when - then에 따라 테스트 코드를 재작성하였습니다.

ICCHOI · 2023-06-12T01:14:30Z

calculator/src/main/java/com/wonu606/calculator/CalculatorApp.java

+    private Strategy getStrategy(int selection) {
+        try {
+            return strategies.get(selection - 1);
+        } catch (IndexOutOfBoundsException exception) {
+            throw new IllegalArgumentException(Message.INVALID_INPUT);
        }
    }


전략을 가져오는 메소드가 잘못된 입력값에 관한 에러를 던지는 것은 부자연스러워 보여요!

따로 빼보려고 하였으나(getStrategy() -> checkStrategy()), 안에서 처리하는 것이 깔끔할 것 같아 함수명을 변경하였습니다.

변경

getStrategy -> getStrategyOrThrow로 함수명을 변경하였습니다.

strategies에서 index로 가져오는 것이 명시적이지 않아 보여 hashMap으로 변경하였습니다.

ICCHOI · 2023-06-12T01:17:41Z

calculator/src/main/java/com/wonu606/calculator/CalculatorApp.java

+    private static int inputMenuSelection(Input input,
+            Converter<String, Integer> stringIntegerConverter)
+            throws IOException {
+        return stringIntegerConverter.convert(input.getInput());
+    }


throws IOException을 적절하게 처리 하지 못해서 코드 전반적으로 해당 예외가 메소드 시그니처에 반복되고 있어요!

변경

Input로 입력시 throws IOException 처리가 필수가 아니라고 느껴 제거하였습니다.

Scanner 입력시 IOException 처리를 안해주어도 되어 제거하였습니다.

제거 하지 않을 경우

private String inputMenuSelection(Input input) { while (true) { try { return input.getInput(); } catch (IOException e) { e.printStackTrace(); } } }

에러를 뱉고 재입력을 받게 하려고 하였습니다.
하지만 만약에 발생을 하면 그 이유가 자바에서 읽으려는 파일에 있어 위와 같은 코드가 무의미하다고 느꼈습니다.
또한, IOException 처리가 CalculatorApp의 담당이 아니라고 생각하여 어떻게 해야 할 지를 모르겠습니다.
만약 IOException 처리를 해주어 할 경우 어떻게 처리해 주는 것이 적절한지 궁금합니다!

IOException이 발생하면 재입력을 받을 수 있는 상황일까? 고민해보는 것도 좋아보여요

wonu606 · 2023-06-12T06:10:47Z

이른 아침부터 답변해주셔서 감사합니다!
울산 잘 다녀오셨나요??

최소 단위의 기능을 만족하면 구현 순서는 중요하지 않다고 생각해요! 지금은 Strategy가 많은 로직에 영향을 끼치고 있어서 그렇게 느끼시는게 당연하다고 생각됩니다.

고민을 많이 했었는데 당연한 거였군요..ㅎㅎ 감사합니다!
말씀을 들어보니 커밋 단위는 그대로 하되 pull request 때는 간단한 기능까지 하는 것이 이해하기 편하지 않을까? 하는 생각을 하였습니다.
다음 pull request 때 적용해보겠습니다.

Message 클래스를 추상 클래스로 만들면 어떤 장점이 있나요?

enum을 사용했을 때 getString()을 사용하는 것이 불필요하다 느껴져 추상 클래스로 만들었습니다.
한데, toString()을 이용하면 되고 제가 작성한 메시지가 불변이므로 enum이 적합한 것 같습니다.
Message 클래스에서 추상 클래스로 구현할 경우의 장점은 없는 것 같습니다.

getString()을 불러오는 것이 어떤 면에서 가독성을 헤친다고 생각하셨나요?

Message.INVALID_NUMBER 라고 하면 클래스 이름과 변수명에 '잘못된 숫자시 쓰는 메시지'라는 의미가 담겨있다고 생각하였습니다.
(A == null) == null 처럼 중복된 말을 반복한다고 느껴져 이 부분이 가독성을 헤친다고 생각하였습니다.

Message 클래스를 일반적으로 enum으로 만든다고 하셨는데 이유가 뭘까요?

Enum으로 작성하게 되면 객체의 생성부터 객체의 값 변경까지 모두 불변이므로
Message 같이 특정 집합(메시지라는 의미)을 갖는 객체들을 모아 관리할 수 있고, 값을 변경할 일이 없어 사용한다고 생각하였습니다.

커밋 단위는 작업하기 편한 방식으로 진행 해주셔도 될 것 같습니다.

넵!! 감사합니다.

즐겁고 행복한 한주 되세요!!

devcourse-ray

고생하셨습니다~
몇 가지 리뷰 작성해뒀으니 확인 해주세요~

입출력 다음 어떤 구현을 먼저 했어야 다른 사람들이 코드를 이해하기 좋을지 궁금합니다.
Strategy 인터페이스부터 만들었는데, 이 코드를 어디에 쓰려는 건지 나만 알 수도 있겠다는 생각이 들었습니다.

구현 순서가 정해져있는건 아니지만 제가 한다면 입출력 이후에는 히스토리 조회하는 인터페이스 실행, 수식 계산하는 인터페이스 실행 이정도로 올릴 것 같아요. 말씀하신대로 Strategy 패턴을 어떻게 사용하려는지 목적이 나와있지 않아서 복잡해 보이는 부분이 있네요.
Message 클래스를 enum이 아닌 abstract class로 만들었습니다.
일반적으로 enum으로 만드는 거 같은데 전 getString()을 불러오는 것이 가독성을 헤친다고 생각하였습니다.
이렇게 써도 될까요?

추상 클래스가 어떤 특성을 가지고있는지 고민해보면 좋을 것 같아요. 그리고 enum 클래스를 사용하는 부분이 정말로 가독성을 해치는 부분인가도 같이 고민해볼 포인트로 보입니다!
점점 코드가 커지다 보니 연관 관계인 클래스들이 많아졌습니다.
커밋 단위로 나누기에도 애매하고 커밋 메시지에 구현 여부를 굳이 써야 하나라는 생각이 들었습니다.
제 코드에서는 CalculationResult 객체입니다. 일반적인 예시는 Dto 역할만을 수행하는 클래스입니다.

사실 저는 PR 볼 때는 커밋 단위를 하나하나 보지는 않아서 어떻게 올라오든 큰 영향은 없어요. 하지만 특정 부분을 어떤 흐름으로 개발했는지 볼 때 한 번씩 커밋을 따라가보기 때문에 PR 올리기 전 코드 정리한다는 느낌으로 기능 단위로 묶어서 올려주시면 좀 더 깔끔할 것 같아요.

devcourse-ray · 2023-06-12T13:59:40Z

calculator/src/main/java/com/wonu606/calculator/CalculatorApp.java

+    public CalculatorApp() {
+        initStrategies();
+    }
+
+    private void initStrategies() {
+        // TODO 조회와 계산 기능을 생성하여 추가해야 함
+    }


전략은 어떻게 받아오나요??

전략은 객체내에서 직접 생성합니다!!

calculator/src/main/java/com/wonu606/calculator/CalculatorApp.java

devcourse-ray · 2023-06-12T14:01:20Z

calculator/src/main/java/com/wonu606/calculator/model/CalculationResult.java

+    @Override
+    public boolean equals(Object o) {
+        if (this == o) {
+            return true;
+        }
+        if (o == null || getClass() != o.getClass()) {
+            return false;
+        }
+        CalculationResult that = (CalculationResult) o;
+        return Double.compare(that.getResult(), getResult()) == 0 && Objects.equals(
+                getExpression(), that.getExpression());
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(getExpression(), getResult());
+    }


기본 equals와 hashCode 는 어떻게 구현되어있을까요?

ICCHOI

원우님..! 올려주신 PR의 양이 많아서 리뷰의 갯수도 많네요!
이전에 리뷰 해드린 부분이 수정이 안된 부분도 있어서, 적극적으로 코멘트를 남겨주시면서 진행하면 좋겠다는 생각이 들어요 :)
또, force-push로 commit한 내용을 올리실 필요 없이, commit 후 push 하면 수정한 commit들이 바로 반영됩니다!

ICCHOI · 2023-06-15T14:51:21Z

calculator/src/main/java/com/wonu606/calculator/CalculatorApp.java


 public class CalculatorApp implements App {

-    Input input;
-    Print printer;
+    private final Map<String, CalculatorStrategy> strategies = new HashMap();


Map으로 CalculatorStrategy를 관리하는 것의 장점이 무엇인가요?

메뉴 번호와 기능 자체를 한 곳에서 모아두고 관리하고 싶어 사용하였습니다.
만약 메뉴 번호도 추가되고 기능 자체도 계산기앱에 추가될 경우 Map.put()만으로 추가하는 장점이 있다고 생각합니다!

ICCHOI · 2023-06-15T14:51:41Z

calculator/src/main/java/com/wonu606/calculator/CalculatorApp.java

+        strategies.put("1", new CalculationStrategy(
+                new InfixValidator(), new InfixToPostfixConverter(), new PostfixCalculator()));


구현체에 직접 의존하게 된다면,� 객체지향의 장점을 살리기 힘들 것 같은데 어떻게 생각하시나요?

ICCHOI · 2023-06-15T14:54:00Z

calculator/src/main/java/com/wonu606/calculator/CalculatorApp.java

+    private CalculatorStrategy getStrategyOrThrow(String selection) {
+        CalculatorStrategy selectedStrategy = strategies.get(selection);
+        if (selectedStrategy == null) {
+            throw new IllegalArgumentException(CalculatorMessage.INVALID_INPUT.message);
+        }
+        return selectedStrategy;
+    }


OrThrow라는 이름은 특정 상황이 만족하면 예외를 던질 수도 있다고 느껴져요.
�getStrategy()로 충분할 것 같습니다.

ICCHOI · 2023-06-15T14:55:06Z

calculator/src/main/java/com/wonu606/calculator/calculator/PostfixCalculator.java

+
+    private void handleToken(Deque<Double> operandStack, String token) {
+        if (isOperator(token)) {
+            Operator operator = Objects.requireNonNull(Operator.get(token));


Objects.requireNonNull()로 검사하기 보다는, Operator에 책임을 두는편이 더 좋아보여요

ICCHOI · 2023-06-15T14:56:18Z

calculator/src/main/java/com/wonu606/calculator/calculator/PostfixCalculator.java

+        double secondOperand = operandStack.pop();
+        double firstOperand = operandStack.pop();


stack이 비어있다면 어떻게 되나요?

ICCHOI · 2023-06-15T15:23:51Z

calculator/src/main/java/com/wonu606/calculator/validator/InfixValidator.java

+    private Boolean isNumeric(String str) {
+        try {
+            Double.parseDouble(str);
+        } catch (NumberFormatException e) {
+            return false;
+        }
+        return true;
+    }


isNumeric은 숫자인지 판별하는 메소드라고 기대하게 되는데, 실제로 하는 일은 Double형 parsing이네요..

ICCHOI · 2023-06-15T15:38:35Z

calculator/src/main/java/com/wonu606/calculator/converter/InfixToPostfixConverter.java

+            Deque<String> operatorStack, List<String> postfixExpression, String tokenOperator) {
+        while (!operatorStack.isEmpty()
+                && isHigherOrEqualPrecedence(operatorStack.peek(), tokenOperator)) {
+            postfixExpression.add(operatorStack.pop());


Suggested change

postfixExpression.add(operatorStack.pop());

String operator = operatorStack.pop();

postfixExpression.add(operator);

되도록 가독성을 위해 분리해주세요

ICCHOI · 2023-06-15T15:46:53Z

calculator/src/main/java/com/wonu606/calculator/converter/InfixToPostfixConverter.java

+        Deque<String> operatorStack = new ArrayDeque<>();
+        List<String> postfixExpression = new ArrayList<>();
+
+        String[] tokens = infixExpression.split("\\s");


만약 입력 요구사항이 달라지면 split은 어떻게 되나요?

ICCHOI · 2023-06-15T15:47:41Z

calculator/src/main/java/com/wonu606/calculator/converter/InfixToPostfixConverter.java

+        while (!operatorStack.isEmpty()
+                && isHigherOrEqualPrecedence(operatorStack.peek(), tokenOperator)) {


processTokenIntoPostfix부터 반복문이 굉장히 많은데, 성능적인 이슈는 없을까요?

ICCHOI · 2023-06-15T15:50:20Z

calculator/src/main/java/com/wonu606/calculator/strategy/CalculationStrategy.java

+            store.saveResult(new CalculationResult(inputExpression, result));
+            printCalculationResult(printer, result);


calculate외의 로직도 try-catch로 묶을 필요가 있을까요?

wonu606 added Team-Ray 🔥 매운맛 labels Jun 11, 2023

wonu606 requested review from ICCHOI and devcourse-ray June 11, 2023 18:01

wonu606 self-assigned this Jun 11, 2023

wonu606 force-pushed the main branch 2 times, most recently from 3c04913 to 143213d Compare June 11, 2023 18:33

feat: 조회와 계산 기능을 담을 인터페이스 구현

6fa2360

wonu606 force-pushed the main branch 3 times, most recently from 53f25bb to ef9dcaa Compare June 11, 2023 19:45

ICCHOI requested changes Jun 12, 2023

View reviewed changes

wonu606 force-pushed the main branch from ef9dcaa to 89c4531 Compare June 12, 2023 07:36

test: 저장소 구현체 테스트 작성

490ce1d

wonu606 force-pushed the main branch from 89c4531 to 9a5ca2e Compare June 12, 2023 08:51

devcourse-ray reviewed Jun 12, 2023

View reviewed changes

wonu606 requested a review from ICCHOI June 12, 2023 14:32

wonu606 force-pushed the main branch from 7f0ea39 to fde3da0 Compare June 12, 2023 15:35

wonu606 added 2 commits June 13, 2023 02:41

feat: 저장소 구현체, 메시지를 담을 클래스 구현

728fcad

refactor: Strategy 인터페이스 -> CalculatorStrategy 로 이름 변경

b99177b

wonu606 force-pushed the main branch 2 times, most recently from b214d18 to f97bf72 Compare June 12, 2023 18:35

wonu606 marked this pull request as draft June 13, 2023 12:45

wonu606 added 5 commits June 14, 2023 14:18

refactor: CalculatorApp 객체에서 Input, Print 필드 제거 및 함수화

bac3c9f

refactor: 입력시 throw IOException 예외 명시 삭제

463f199

feat: 연산자 구현

8a83609

feat: 중위 연산식 유효 검사 클래스 구현

72db880

test: 중위 연산식 유효 검사 클래스 테스트 작성 및 통과

8f347a2

wonu606 force-pushed the main branch from f97bf72 to 8f347a2 Compare June 14, 2023 05:19

wonu606 force-pushed the main branch 4 times, most recently from 3bfc0e2 to 77f21b7 Compare June 15, 2023 02:09

wonu606 marked this pull request as ready for review June 15, 2023 02:11

wonu606 added 2 commits June 15, 2023 13:49

feat: 중위 연산식을 후위 연산식으로 바꾸는 converter 구현

95306f6

test: 중위 연산식을 후위 연산식으로 변환하는 Converter 테스트 작성 및 통과

400b2a4

wonu606 force-pushed the main branch from 77f21b7 to 0876608 Compare June 15, 2023 04:49

wonu606 added 5 commits June 15, 2023 13:50

feat: 후위 연산식을 계산하는 클래스 구현

60a1eb4

test: 후위 연산식을 계산하는 클래스 테스트 작성 및 통과

3dfc78e

chore: 불필요한 import문 제거 및 디렉토리 재정렬

ef0ddd8

test: 중위 연산식을 입력 받아 계산하는 클래스 테스트 작성

52ddf84

feat: 계산하는 구현체 구현 및 테스트 통과

7bba45d

wonu606 force-pushed the main branch from 0876608 to 7bba45d Compare June 15, 2023 04:50

ICCHOI requested changes Jun 15, 2023

View reviewed changes

		strategies.put("1", new CalculationStrategy(
		new InfixValidator(), new InfixToPostfixConverter(), new PostfixCalculator()));

		double secondOperand = operandStack.pop();
		double firstOperand = operandStack.pop();

	postfixExpression.add(operatorStack.pop());
	String operator = operatorStack.pop();
	postfixExpression.add(operator);

		while (!operatorStack.isEmpty()
		&& isHigherOrEqualPrecedence(operatorStack.peek(), tokenOperator)) {

		store.saveResult(new CalculationResult(inputExpression, result));
		printCalculationResult(printer, result);

[4기 -유원우] 1~2주차 과제 : 계산기 구현 미션 제출합니다. #167

Are you sure you want to change the base?

[4기 -유원우] 1~2주차 과제 : 계산기 구현 미션 제출합니다. #167

Conversation

wonu606 commented Jun 11, 2023 • edited Loading

📌 과제 설명

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

✅ 피드백 반영사항

✅ PR 포인트 & 궁금한 점

ICCHOI left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wonu606 Jun 12, 2023 • edited Loading

Choose a reason for hiding this comment

재구현 이유

사용

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Object.equals

Object.hashCode

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

변경

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wonu606 Jun 12, 2023 • edited Loading

Choose a reason for hiding this comment

변경

Choose a reason for hiding this comment

wonu606 Jun 12, 2023 • edited Loading

Choose a reason for hiding this comment

변경

Choose a reason for hiding this comment

Choose a reason for hiding this comment

변경

Choose a reason for hiding this comment

Choose a reason for hiding this comment

변경

Choose a reason for hiding this comment

wonu606 commented Jun 12, 2023

devcourse-ray left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ICCHOI left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wonu606 commented Jun 11, 2023 •

edited

Loading

ICCHOI left a comment •

edited

Loading

wonu606 Jun 12, 2023 •

edited

Loading

wonu606 Jun 12, 2023 •

edited

Loading

wonu606 Jun 12, 2023 •

edited

Loading