[4기 - 김재웅] 1~2주차 과제 : 계산기 구현 미션 제출합니다. #147

kju2405 · 2023-06-09T13:54:58Z

📌 과제 설명

계산기능을 수행하는 Calculate, 사용자로 부터 입력을 받는 Input, 사용자에게 보여주는 Output, 계산 기록을 저장하는 Repository 패키지를 각각 만들었습니다.
기존에 구현한 코드를 보다 더 객체지향적인 코드로 변경하려고 노력하였습니다.
예제와 같이 올바르게 입력된 상황이라는 가정하에 구현하였고 그 외의 예외처리에 대해서는 아직 하지 않았습니다. 구조가 잡힌다면 바로 구현하도록 하겠습니다.

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

✅ 피드백 반영사항

Main.java

"변수의 불변성을 보장해주세요"-> private static int choice; 로 처리하였는데 이렇게 하는 방법이 맞는지 궁금합니다.
"인터페이스가 아니라 구현체를 직접 사용하는게 적절한지 생각해보세요" -> 추상화에 의존하기 위하여 코드를 수정하였습니다.
"계산 한 번 할 때마다 계산기를 계속 생성해야할까요?" -> 처음 한번만 생성되게끔 하였습니다.
"비슷한 기능을 하는 메소드들을 다른 객체로 추출하는 건 어떨까요?" -> 다른 객체로 추출하였습니다.

Calculate.java

"계산기의 기능을 다시 고민해보면 좋을 것 같네요" / "조금 더 추상화가 필요하다고 할 수 있겠네요 ㅎㅎ! 기능에 따라 여러 역할로 객체를 나누어 보는 건 어떨까요?" -> 계산기 구현체에서는 식을 입력받고 결과값을 반환하는것 외의 기능은 보이지 않게끔 수정하였습니다.

ExpressionRepository.java

"복수형을 반환하면 getRecords()가 좀 더 좋아보이네요" -> 수정하였습니다!
"요구사항 그대로 값을 저장하는 것도 좋지만,추후에 출력되어야 할 값이 바뀔 경우를 고려해보는 건 어떨까요?" -> 이 부분은 조금 더 고민해보겠습니다😭

UserInterfaceImpl.java

"값을 가져오는 것과, 값을 출력하는 기능은 여러 기능을 한 번에 수행하는 것 처럼 보여요. " -> 기능 분리하였습니다.

✅ PR 포인트 & 궁금한 점

CalOrderImpl.java파일에서 calculateMultiplyDivide 메서드와 calculatePlusMinus메서드를 어떻게 테스트할 수 있을까? 고민하며 리펙토링을 해보려 하였습니다. 하지만 지금 로직을 테스트하기 좋게 어떻게 고칠까에 대한 답은 내리지 못하였습니다. 그래서 이렇게 테스트 코드를 작성하는것은 비효율적인가? 라는 생각을 가지며 테스트 코드를 작성하였습니다. 작성하는 과정에서 테스트코드를 위해 calculateMultiplyDivide메서드의 반환타입을 String으로 변경한것이 찝찝하긴 합니다.

제가 궁금한점은.. 계산하는 로직 부분을 어떻게 고치는게 좋을지 잘 모르겠습니다.....

devcourse-ray · 2023-06-10T12:29:00Z

calculator/src/main/java/org/example/CalOrder.java

+    public abstract void calPriorityFirst();
+    public abstract void calPrioritySecond();


interface의 메서드는 기본적으로 추상 메서드이기 때문에 abstract가 없어도 됩니다.

devcourse-ray · 2023-06-10T12:37:17Z

calculator/src/main/java/org/example/Main.java

+        int choice = 0;
+
+        while (true) {
+            choice = getChoice();


변수의 불변성을 보장해주세요

devcourse-ray · 2023-06-10T12:37:54Z

calculator/src/main/java/org/example/Main.java

+            choice = getChoice();
+            System.out.println();
+            if (choice == 1) {
+                new UserInterfaceImpl().showRecord();


인터페이스가 아니라 구현체를 직접 사용하는게 적절한지 생각해보세요

devcourse-ray · 2023-06-10T12:38:16Z

calculator/src/main/java/org/example/Main.java

+                new UserInterfaceImpl().showRecord();
+            } else {
+                String expression = typeExpression();
+                new Calculate(expression).calculate();


계산 한 번 할 때마다 계산기를 계속 생성해야할까요?

devcourse-ray · 2023-06-10T12:39:11Z

calculator/src/main/java/org/example/Calculate.java

+
+import java.util.Stack;
+
+public class Calculate implements CalOrder{


계산기의 기능을 다시 고민해보면 좋을 것 같네요

주말인데 불구하고 리뷰 남겨주셔서 감사합니다!😄
남겨주신 리뷰 확인하며 수정하는도중에 궁금한 사항이 생겨서요.
혹시 나누기 연산을 시행했을때의 기능 말씀이신지 아니면 전체적으로 효율적이지 못한(?) 계산기 수행 로직이라 전체적인 변경이 필요한지 궁금해서요!
감사합니다!

기본적으로 계산기는 사칙연산을 연산 규칙에 맞춰서 제공해주는 기계라고 봐야해요.
그렇다면 사용자가 마음대로 연산 규칙을 변경해서는 안되겠죠??
사용자가 연산 순서를 바꾸고 싶다면 계산기를 바꾸는게 아니라 연산 식을 새롭게 입력하는 걸 생각해보시면 될 것 같아요.

그렇게 하려면 수식입력, 계산 두 기능만 외부 오픈을 하고 그 외의 모든 내용은 내부적으로 감춰줘야해요.
계산기라는 객체를 실제 계산기라고 봤을 때 수식입력, 계산 외에 다른 기능이 외부에 보인다면 잘못 사용될 여지가 있겠네요.

아하 자세하게 설명해주셔서 감사합니다!
말씀해주신 부분 생각해서 수정해보도록 하겠습니다!

조금 더 추상화가 필요하다고 할 수 있겠네요 ㅎㅎ!
기능에 따라 여러 역할로 객체를 나누어 보는 건 어떨까요?

ICCHOI

안녕하세요 재웅님! 리뷰가 늦어졌네요 :(
전체적으로 로직이 잘 들어나는 점은 좋았어요!
하지만 좀 더 객체지향적으로 리팩토링 해보시면 더 좋겠다는 생각이 들었습니다 ;)

ICCHOI · 2023-06-11T16:07:50Z

calculator/src/main/java/org/example/Calculate.java

+
+import java.util.Stack;
+
+public class Calculate implements CalOrder{


조금 더 추상화가 필요하다고 할 수 있겠네요 ㅎㅎ!
기능에 따라 여러 역할로 객체를 나누어 보는 건 어떨까요?

ICCHOI · 2023-06-11T16:11:58Z

calculator/src/main/java/org/example/ExpressionRepository.java

+    public List<String> getRecord() {
+        return store;
+    }


복수형을 반환하면 getRecords()가 좀 더 좋아보이네요

ICCHOI · 2023-06-11T16:12:33Z

calculator/src/main/java/org/example/ExpressionRepository.java

+
+    @Override
+    public void save(String expression, int result) {
+        store.add(expression + " = " + result);


요구사항 그대로 값을 저장하는 것도 좋지만,
추후에 출력되어야 할 값이 바뀔 경우를 고려해보는 건 어떨까요?

ICCHOI · 2023-06-11T16:14:06Z

calculator/src/main/java/org/example/Main.java

+    public static int getChoice() {
+        Scanner scanner = new Scanner(System.in);
+
+        System.out.println("1. 조회\n2. 계산\n");
+        System.out.print("선택 : ");
+        return scanner.nextInt();
+    }
+
+    public static String typeExpression() {
+        Scanner scanner = new Scanner(System.in);
+        return scanner.nextLine();
+    }


비슷한 기능을 하는 메소드들을 다른 객체로 추출하는 건 어떨까요?

ICCHOI · 2023-06-11T16:15:29Z

calculator/src/main/java/org/example/UserInterfaceImpl.java

+    public void showRecord() {
+        List<String> record = repository.getRecord();
+        record.forEach(System.out::println);
+    }


값을 가져오는 것과, 값을 출력하는 기능은 여러 기능을 한 번에 수행하는 것 처럼 보여요.

ICCHOI

재웅님! 수고하셨습니다.
전체적으로 절차지향적으로 짜여있는 부분이 많은 거 같아요.
객체지향 생활체조 원칙을 공부해보시고 이 원칙을 지키면서 수정해보시면 좋을 것 같아요.

ICCHOI · 2023-06-14T07:24:49Z

calculator/src/main/java/org/example/CalConfig.java

+    public Calculate calculate(){
+        return new CalculateImpl(new CalOrderImpl());
+    }
+    public Repository repository(){
+        return new ExpressionRepository();
+    }
+    public Input input(){
+        return new UserInput();
+    }
+    public Show show(){
+        return new ShowText();
+    }
+    public PreProcess preProcess() {
+        return new PreProcessImpl();
+    }


Config를 통해 객체를 생성하면 어떤 장점이 있나요?

ICCHOI · 2023-06-14T07:25:36Z

calculator/src/main/java/org/example/Calculate/CalOrder.java

+    void calMultiplyDivide();
+    int calPlusMinus();


Suggested change

void calMultiplyDivide();

int calPlusMinus();

void calculateMultiplyDivide();

int calculatePlusMinus();

메소드 이름이 길어지더라도 명시적으로 작성해주세요!

ICCHOI · 2023-06-14T07:27:39Z

calculator/src/main/java/org/example/Output/ShowText.java

+
+import java.util.List;
+
+public class ShowText implements Show{


클래스 이름은 명사로 만들어주세요

ICCHOI · 2023-06-14T07:28:48Z

calculator/src/main/java/org/example/Main.java


        while (true) {
-            choice = getChoice();
+            show.showMenu();
+            choice = input.inputChoice();
            System.out.println();
            if (choice == 1) {


choice 1이 어떤 동작인지 어떻게 알 수 있을까요?

ICCHOI · 2023-06-14T07:29:07Z

calculator/src/main/java/org/example/Main.java


        while (true) {
-            choice = getChoice();
+            show.showMenu();
+            choice = input.inputChoice();
            System.out.println();


이것도 출력을 담당하는 객체에서 수행하면 좋을 것 같아요!

멘토님, 리뷰 감사드립니다!
혹시 show.showMenu()부분을 말씀하시는걸까요???

System.out.println();으로 하드코딩 되어 있는 부분입니다 :)

아하 감사합니다🙇‍♂️

ICCHOI · 2023-06-14T10:45:33Z

calculator/src/main/java/org/example/Calculate/CalOrderImpl.java

@@ -35,7 +20,7 @@ public void calPriorityFirst() {
    }

    @Override
-    public void calPrioritySecond() {
+    public int calPlusMinus() {


이 메서드를 어떻게 테스트 할 수 있을까? 라는 질문을 하면서 리팩토링을 해보시면 좋을 것 같아요.

ICCHOI · 2023-06-14T10:47:47Z

calculator/src/main/java/org/example/Calculate/PreProcessImpl.java

+    Stack<String> expressionStack = new Stack<>();
+    @Override


Suggested change

Stack<String> expressionStack = new Stack<>();

@Override

Stack<String> expressionStack = new Stack<>();

@Override

ICCHOI · 2023-06-14T10:52:35Z

calculator/src/main/java/org/example/Calculate/PreProcessImpl.java

+    public Stack<String> expressionToStack(String expression) {
+        this.expression = expression;
+        String[] expressionArr = expression.split(" ");
+        expressionStack.clear();
+        for (String inputVal : expressionArr) {
+            expressionStack.add(inputVal);
+        }
+        return expressionStack;
+    }


너무 요구사항과 강하게 결합 되어 있는 것 같아서, 재사용성을 고려해보는 것도 좋을 거 같아요 :)

ICCHOI · 2023-06-14T10:58:13Z

calculator/src/main/java/org/example/Input/UserInput.java

+    Scanner scanner = new Scanner(System.in);
+    @Override
+    public int inputChoice() {
+        return scanner.nextInt();


inputChoice()라는 메서드명과 실제로 하고 있는 일이 조금 달라보여요.

devcourse-ray

인창님께서 대부분 리뷰를 남겨주셔서 거의 중복되는 내용이라 따로 더 남기지는 않았어요
리뷰 내용에 맞춰서 수정 후에 다시 확인할게요~

devcourse-ray · 2023-06-14T14:29:12Z

calculator/src/main/java/org/example/Calculate/CalOrderImpl.java

+    public void multiply(int idx) {
+        result = Integer.parseInt(expressionStack.get(idx - 1)) * Integer.parseInt(expressionStack.get(idx + 1));
+        expressionStack.add(idx - 1, String.valueOf(result));
+        expressionStack.remove(idx);
+        expressionStack.remove(idx);
+        expressionStack.remove(idx);
+    }
+
+    public void divide(int idx) {
+        result = Integer.parseInt(expressionStack.get(idx - 1)) / Integer.parseInt(expressionStack.get(idx + 1));
+        expressionStack.add(idx - 1, String.valueOf(result));
+        expressionStack.remove(idx);
+        expressionStack.remove(idx);
+        expressionStack.remove(idx);
+    }


내부에서만 사용하는 메소드는 접근제어자 설정을 잘 해줘야해요

kju2405 added 3 commits June 9, 2023 17:44

feat: 사칙연산 기능 구현

1c5c20b

feat: 저장소 구현

31d64fe

feat: 조회 기능 추가

d42a6d9

kju2405 added Team-Ray 🔥 매운맛 labels Jun 9, 2023

kju2405 requested review from ICCHOI and devcourse-ray June 9, 2023 13:54

devcourse-ray requested changes Jun 10, 2023

View reviewed changes

kju2405 self-assigned this Jun 11, 2023

ICCHOI requested changes Jun 11, 2023

View reviewed changes

kju2405 marked this pull request as draft June 13, 2023 23:20

kju2405 added 13 commits June 14, 2023 08:34

style: Interface 메서드 제어자 삭제

b36e927

refactor: 메뉴 출력 기능 분리

c0c1a07

refactor: 입력하는 기능 분리

14789d6

style: Record -> Records

ef5eeca

refactor: 출력할 결과들을 가져오는 기능과 보여주는 기능 분리

1bd816a

refactor: Output 패키지의 Show인터페이스 파일로 합침

2b75683

refactor: 기능별 패키지 생성

8e7fc67

refactor: type보다 input이 더 명확한것 같아 변경

2324418

refactor: 결과 출력 기능 분리

71917d0

refactor: 입력받은 식 전처리 작업 기능 분리

15ffeda

refactor: 사칙연산의 기능만 수행하도록 분리

50b581d

refactor: 식을 입력받고 결과값을 반환하는 기능만 수행, 나머지 기능은 내부적으로 감춤

d31597e

refactor: DI컨테이너 생성

057119f

kju2405 marked this pull request as ready for review June 14, 2023 06:20

kju2405 requested review from devcourse-ray and ICCHOI June 14, 2023 06:20

ICCHOI requested changes Jun 14, 2023

View reviewed changes

devcourse-ray reviewed Jun 14, 2023

View reviewed changes

style: 명확하게 이름 변경

8e49f93

kju2405 added 5 commits June 16, 2023 22:25

refactor: 공통부붐 메서드 추출

1429b49

refactor: enum추가하여 각 메뉴 선택을 명확하게 함

0f7a612

refactor: 개행 문자 객체에서 수행

c22e1c6

refactor: stream으로 변환

4f44f1b

test: 사칙연산 테스트 코드 작성

f8ef218

kju2405 requested review from devcourse-ray and ICCHOI June 16, 2023 13:51

kju2405 added 4 commits June 17, 2023 00:09

test: 테스트 코드 삭제

63e778c

refactor: 계산 순서는 내부적으로 숨김

8f97f36

style: CalculateImpl -> Calculator

eee0518

test: 사칙연산 우선순위 테스트 코드 작성

cb39427

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[4기 - 김재웅] 1~2주차 과제 : 계산기 구현 미션 제출합니다. #147

[4기 - 김재웅] 1~2주차 과제 : 계산기 구현 미션 제출합니다. #147

kju2405 commented Jun 9, 2023 •

edited

Loading

devcourse-ray Jun 10, 2023

devcourse-ray Jun 10, 2023

devcourse-ray Jun 10, 2023

devcourse-ray Jun 10, 2023

devcourse-ray Jun 10, 2023

kju2405 Jun 11, 2023

devcourse-ray Jun 11, 2023

kju2405 Jun 11, 2023

ICCHOI Jun 11, 2023

ICCHOI left a comment

ICCHOI Jun 11, 2023

ICCHOI Jun 11, 2023

ICCHOI Jun 11, 2023

ICCHOI Jun 11, 2023

ICCHOI Jun 11, 2023

ICCHOI left a comment

ICCHOI Jun 14, 2023

ICCHOI Jun 14, 2023

ICCHOI Jun 14, 2023

ICCHOI Jun 14, 2023

ICCHOI Jun 14, 2023

kju2405 Jun 15, 2023

ICCHOI Jun 15, 2023

kju2405 Jun 15, 2023 •

edited

Loading

ICCHOI Jun 14, 2023

ICCHOI Jun 14, 2023

ICCHOI Jun 14, 2023

ICCHOI Jun 14, 2023

devcourse-ray left a comment

devcourse-ray Jun 14, 2023

		public abstract void calPriorityFirst();
		public abstract void calPrioritySecond();


		import java.util.Stack;

		public class Calculate implements CalOrder{


		import java.util.List;

		public class ShowText implements Show{

[4기 - 김재웅] 1~2주차 과제 : 계산기 구현 미션 제출합니다. #147

Are you sure you want to change the base?

[4기 - 김재웅] 1~2주차 과제 : 계산기 구현 미션 제출합니다. #147

Conversation

kju2405 commented Jun 9, 2023 • edited Loading

📌 과제 설명

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

✅ 피드백 반영사항

Main.java

Calculate.java

ExpressionRepository.java

UserInterfaceImpl.java

✅ PR 포인트 & 궁금한 점

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

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

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

kju2405 Jun 15, 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

devcourse-ray left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kju2405 commented Jun 9, 2023 •

edited

Loading

kju2405 Jun 15, 2023 •

edited

Loading