[치킨 POS 미션] 고준보 미션 제출합니다. #2

Ridealist · 2022-12-12T09:06:31Z

No description provided.

결제 금액을 계산하는 Pay 클래스 추가 PayMethod 클래스에 숫자에 의해 PayMethod 연결해주는 메소드 추가 OrderRepository에 특정 table의 모든 order들을 조회하는 메소드 추가

… 로직화

khj1

안녕하세요 준보님 다시 뵙네요 ㅎㅎ
간단하게 리뷰 남겨봤습니다!

개인적으로 네이밍이 너무 적절하고 로직도 깔끔해서 코드를 따라가기 정말 수월했습니다 👍
다만 비즈니스 로직에 getter() 가 꽤나 많이 사용된 것이 조금 아쉬웠습니다.
조금만 의식하신다면 훨씬 좋은 코드를 작성하실 수 있을거라 생각해요!

시간 나실 때 제 코드도 한번 리뷰 부탁드립니다 :)

khj1 · 2022-12-13T00:55:27Z

src/main/java/domain/order/OrderRepository.java

+
+    public static boolean hasOrdersOnTable(final Table table) {
+        return orders.stream()
+                .anyMatch(order -> order.getTable().equals(table));


이 부분은 getter 로 값을 불러오지 않고 처리할 수 있을 것 같습니다.

네 동의 합니다. getter를 줄이도록 의식적으로 노력을 많이 해야겠네요:)

khj1 · 2022-12-13T00:57:29Z

src/main/java/domain/order/OrderRepository.java

+                .filter(order -> order.getTable().equals(table))
+                .filter(order -> order.getMenu().equals(menu))


Order 객체에서 equals, hashCode 메서드를 재정의해서 사용해보시는 것도 괜찮을 것 같습니다!

오 좋은 지적 감사합니다. Order에서 메서드를 재정의 하면, getter 없이도 equals로 필터링을 할 수 있겠네요:)

khj1 · 2022-12-13T01:13:06Z

src/main/java/domain/pay/Pay.java

+    private static final int DISCOUNT_PRICE = 10_000;
+
+    private final Table table;
+    private final List<Order> orders;


일급 컬렉션을 사용해보시는 것도 추천 드립니다!

khj1 · 2022-12-13T01:48:38Z

src/main/java/domain/pay/Pay.java

+    // TODO 리팩터링 하기!
+    public Map<Menu, Integer> aggregateMenus() {
+        Map<Menu, Integer> menusCounts = new HashMap<>();
+        for (Menu menu : getUniqueMenus()) {
+            menusCounts.put(menu, countMenusFormOrders(menu));
+        }
+        return menusCounts;
+    }


같은 테이블에 같은 메뉴가 추가되는 경우를 OrderRepository 에서 처리해주시는 것은 어떨까요?

네 저도 정보가 order라는 단위로 너무 흩어지는게 문제라고 생각했었습니다...
말씀하신 것처럼 같은 테이블에 같은 메뉴가 추가되는 것을 별도의 order 객체를 생성하는 것이 아닌,
OrderRepository에서 묶어 저장하는 방식을 고민해보겠습니다. 감사합니다!

src/main/java/domain/pay/PayMethod.java

khj1 · 2022-12-13T02:11:38Z

src/test/java/domain/OrderTest.java

+        Table table = TableRepository.findTableByNumber(1);
+        Menu menu = MenuRepository.findMenuByNumber(1);


@BeforeEach 로 코드 중복을 줄일 수 있을 것 같습니다!

khj1 · 2022-12-13T02:13:44Z

src/test/java/ApplicationTest.java

+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+class ApplicationTest extends NsTest {


와... ApplicationTest 도 직접 추가하셨네요 진짜 멋있습니다 👍👍👍👍

칭찬 감사합니다:)

khj1 · 2022-12-13T02:36:31Z

src/main/java/view/OutputView.java

        System.out.println();
    }
+
+    public static void printOrderList(final Pay pay) {


출력에 필요한 메뉴 이름, 주문 수량, 메뉴 가격 만 추출해서 전달해보시는 것은 어떨까요?

생각해보니 이건 한준님 말씀처럼 데이터만 넘기는게 적절하겠네요.
객체에 대한 직접 참조를 한다면 View 내부에서 Model에 대한 의존성이 생겨버리는 것이니, Controller에서 Model에 대한 처리를 모두 하고 그 결과값 데이터만 VIew에 넘기는 방식으로 설계하는게 더 타당한 것 같습니다.
조언 감사합니다:)

khj1 · 2022-12-13T02:45:26Z

src/main/java/view/InputView.java

+        return validateNumber(input);
+    }
+
+    public static int inputMenuChoice() throws IllegalArgumentException {


예외를 throws 해주신 이유가 뭔지 알 수 있을까요? 궁금합니다!

@khj1 현재 메서드에서 IllegalArgumentException를 발생시키는 코드가 있다는 것을 알리려는 것 같습니다!

메서드 시그니처에 throws를 명시해주면, IntelliJ에서 해당 메서드를 다른 클래스에서 활용할 때 exception 발생 여부를 알려주어 그렇습니다. controller단에서 빼먹지 않고 exception을 처리하기 위한 표기 용도로 보시면 됩니다:)

khj1 · 2022-12-13T02:48:33Z

src/main/java/domain/pay/Pay.java

+    private int getOriginalTotalCost() {
+        List<Order> orders = OrderRepository.getOrdersByTable(table);
+        return orders.stream()
+                .map(order -> order.getMenu().getPrice() * order.getMenuAmount())


주문 당 결제 금액을 계산 하는 책임은 order 가 가지는 것이 좋아 보입니다!

이것도 order에게 책임을 부여했다면 getter를 사용하지 않을 수 있었네요...ㅎㅎ order.calculateCost 이런 식으로요
getter 대신 메세지 보낸다 덕분에 배워갑니다!

gitchannn · 2022-12-13T04:22:33Z

docs/README.md

+    - ex) 10개는 10,000원 할인, 20개는 20,000원 할인
+  - [x] 현금 결제는 5% 할인, 할인된 금액에서 한 번 더 할인이 가능
+- [x] 주문 혹은 결제가 불가능한 경우 그 이유를 보여 주고, 다시 주문 혹은 결제가 가능하도록 한다
+- [x] 최종 결제 금액을 보여준다


개인적 궁금증인데,

저는 뭔가 프로그램을 설계할 때의 기능 목록과
사람이 생각하는 기능에 대한 목록이 다른 것 같거든요

예를 들어서, 다리건너기 게임에서
게임이 종료할 때 총 시도 횟수 출력 이라는 역할은 플레이어의 입장에서의 순서인 것 같고
구현하기 위해서는 시도 횟수 1로 초기화, 시도 회수 +1같은 과정들을 commit하는게 더 편하더라구요

준보님 기능목록을 보면 테이블에 금액에 대한 내용이 들어있지 않아서
아마도 나중에 테이블 세팅을 완료하고 2번 결제에 들어간 후에 금액을 찾아오는 방식인지,
아니면 기능 목록은 플레이어 입장에서 적어놓지만 그때그때 잊지않고 세팅을 하는건지 궁금합니다!

일단, 이번 기능 목록은 너무 대충 작성해 테이블 금액 관련된 내용을 적는 것을 깜박한거지만..ㅎㅎ
은기님 기능 목록처럼 플레이어 입장이 아닌, 구현하는 사람 입장에서 작성하는 것이 맞다고 생각합니다.

위 기능목록은 전체 흐름을 파악하는 용으로만 사용했고, 계속 프로그램 실행 예시를 보면서 필요한 것을 추가해 나가는 식으로 개발했었습니다:)

이것도 저 스스로 고쳐야할 부분이네요... 기능 목록과 실제 구현의 괴리를 좁히는 것이 살아 있는 문서를 만드는 방법이니까요~

gitchannn · 2022-12-13T04:24:48Z

src/main/java/view/InputView.java

+
+    private static int validateNumber(String input) {
+        try {
+            return Integer.parseInt(input.trim());


저는 연산에 필요한 것이 아니라면 꼭 숫자 형태의 문자를 숫자로 전환해야 하나? 에 대한 의문이 있어요
현재 1,2,3으로 입력되는 메인 옵션을 연산에 사용할 것이 아니기 때문에 그대로 String 상태로
List.of("1","2,","3").contains(input) 정도의 검증으로 끝내도 될 것 같아요

생각해보니 그렇네요. 각각의 1,2,3은 선택의 표시이지 그 자체가 값으로의 숫자의 의미가 아니까 변환할 필요가 없겠네요. 좋은 피드백 감사합니다!

gitchannn · 2022-12-13T04:25:44Z

src/main/java/view/InputView.java

+        return validateNumber(input);
+    }
+
+    public static int inputMenuChoice() throws IllegalArgumentException {


@khj1 현재 메서드에서 IllegalArgumentException를 발생시키는 코드가 있다는 것을 알리려는 것 같습니다!

gitchannn · 2022-12-13T04:26:45Z

src/main/java/view/InputView.java

+    private static <T> T breakOneLineAfterRead(Supplier<T> reader) {
+        T input = reader.get();
+        System.out.println();
+        return input;


오.. 구현하라는 예시 코드와 정확하게 일치하게 만드려면 이 코드가 필요하네요...!

gitchannn · 2022-12-13T04:36:38Z

src/main/java/controller/MenuController.java

+        if (command == Command.REGISTER) {
+            PosController.registerOrder();
+        }
+        if (command == Command.QUIT) {


Command에서 나열한 순서대로 REGISTRER, PAY, QUIT 순서로 놓는게 더 일관적일거같아요

gitchannn · 2022-12-13T04:40:06Z

src/main/java/controller/MenuController.java

+
+    private static void validateAnyOrderExist() {
+        if (!OrderRepository.hasAnyOrders()) {
+            throw new IllegalArgumentException("등록된 주문이 하나도 없습니다. 주문을 먼저 등록해주세요.");


exception message를 상수로 분리해주시는게 더 좋을 것 같아요

이 부분 계속 고민을 하다가 exception message는 상수가 아니라고 결론을 내려서 이번에는 따로 상수 선언을 하지 않고 값으로 바로 처리를 해보았습니다. 하지만 아직도 고민이 되네요 이 부분은... 흠...

gitchannn · 2022-12-13T04:41:13Z

src/main/java/controller/PosController.java

+    private static final List<Table> tables = TableRepository.tables();
+    private static final List<Menu> menus = MenuRepository.menus();
+
+    public static void registerOrder() {


PosController 내의 모든 메서드가 다 static인데,
혹시 Util 클래스처럼 사용하려 했던건지 궁금합니다!

PostController가 따로 관리하는 상태(필드)가 존재하는 것도 아니고,
registerOrder() 과 payTable() 의 메서드를 중계해주는 역할을 한다고 생각해서 객체를 생성할 필요가 없는 static 클래스로 설계해 보았습니다:)

gitchannn · 2022-12-13T04:42:07Z

src/main/java/domain/menu/Menu.java

+
+    public Category getCategory() {
+        return category;
+    }


전부 다 getter로 활용하지 말구 이중에 꼭 내용 그대로 출력해야되는 것 아니라면
Category는 isChicken같은 걸로 최대한 적은 정보를 외부에 노출하는게 더 좋을 것 같아요

네 동의합니다. 더 캡슐화를 높이는 게 맞겠네요!

Ridealist added 30 commits December 10, 2022 18:07

docs: 구현할 기능 목록 작성

8def694

fix: gradle java 버전 세팅 변경

e63159f

feat(Order): 주문 클래스 구현

a6d76ab

feat(OrderRepository): 전체 주문 내역 저장 클래스 구현

7c736db

chore: gradle 세팅 변경

41e50d7

feat(View): InputView 및 OutputView 프로그램 첫 실행 화면 구성 메소드 추가

d96ca84

refactor(Order): menuAmount로 인스턴스 필드 이름 수정

a535e97

refactor(OutputView): 메소드 이름 명료화, 줄바꿈 추가

c0101cb

feat(InputView): 주문 등록 기능 관련 메소드 추가

9c94528

feat(Table): Repository에서 테이블 이름 기준으로 테이블 찾는 기능 추가

02d610e

feat(Menu): Repository에서 메뉴 번호 기준으로 메뉴 찾는 기능 추가

206c96d

feat(OrderRepository): 새로운 Order 객체 저장 및 전체 목록 조회 기능 추가

232833d

refactor(InputView): nextstep Console 클래스 적용

7afca7c

feat: Repository에 저장된 모든 객체 ID(number)값 가져오는 기능 구현

762434d

feat: 메인화면 입력값 유효성 검증 로직 구현

e3efc80

feat: 테이블 선택 및 메뉴 등록 기능 유효성 검증 로직 구현

1fe335f

feat(View): 결제하기 기능을 위한 입출력 메서드 구현

6e145c7

feat(Pay): 결제방식에 따른 할인율 Enum 클래스 구현

451e894

fix: 잘못된 입력에 대한 에러 메세지 내용 수정

0d2f81f

feat(Pay): 결제 금액을 계산하는 클래스 구현

a256864

결제 금액을 계산하는 Pay 클래스 추가 PayMethod 클래스에 숫자에 의해 PayMethod 연결해주는 메소드 추가 OrderRepository에 특정 table의 모든 order들을 조회하는 메소드 추가

feat(OutputView): 최종 결제 금액 출력 메서드 구현

681eb52

feat(PosController): 메뉴 기능별 실행 메소드 로직 관리 클래스 구현

8bbea56

feat(Command): 메인 메뉴 선택 관리하는 Enum 클래스 추가

3e98604

refactor(View): Command 클래스 적용해 출력 로직 단순화

fa86222

feat(MenuController): 메인 메뉴 선택 관리하는 클래스 구현

8788d18

refactor(Application): MenuController 클래스 적용

49d84ee

docs: README 현재까지 구현 사항 체크

a5bf6bf

feat(Pay): 주문 내역으로 부터 주문 사항 통계 내는 메서드 구현

6d88b50

refactor: Pay 변경사항 반영

8ac8d69

chore: 필요없는 import 패키지 제거

6123a32

Ridealist added 20 commits December 11, 2022 01:38

feat(OutputView): 주문한 테이블에 대해 표시 기능 구현

39d1f79

fix(PayMethod): 주문 방법에 따른 할인률 오류 수정

4bdc383

fix(OrderRepository): 메뉴 당 개수가 아닌 전체 개수 기준 계산 오류 해결

4167459

fix(OutputView): 결제 시 수량을 곱한 메뉴당 전체 금액이 출력되도록 수정

6ba5156

fix(Pay): 주문 내역이 없는 테이블도 결제 가능한 오류 해결

fc6aa5c

chore: 해결된 TODO 리스트 제거

5a96d9e

docs: 전체 기능 1차 구현 완료_1시간 반정도 오버 됨

c6c2d4e

chore: TODO 리스트 추가

d2aa3fe

refactor(Command): 기능 선택 입력에 대한 유효성 검증 책임 추가 및 메서드 구현

2512285

refactor(InputView): 입력 후 띄어쓰기 기능을 제너릭 메서드로 일괄 처리

ca4f801

refactor(InputView): InputValidator 삭제, 유효성 검증 InputView 내부 메서드 및 도메인…

7c2e1c8

… 로직화

refactor(Repository): 불필요한 메서드 제거, 유효성 검증 로직 내부 메서드에서 구현

6e8ac09

refactor(Order): 최대 주문 가능 수량 유효성 검증 로직 내부화

a5ac038

refactor(Order): 필요 없는 로직 제거, 기능 순서에 따라 메서드 순서 조정

9bdd561

refactor(Command): 입력값 검증 로직 내부화를 위한 파라미터 타입 변경

b8c352d

refactor(PosController): 메서드 분리, 메서드 이름 명료화, 트랜잭션 단위 기준 메서드 구현

fcd1f3b

refactor(Pay): 논리적 흐름에 따라 메서드 순서 조정

f9567da

refactor(OutputView): 메서드 파라미터 변경 적용, 에러 메세지 이후 한 줄 띄어쓰기 추가

77c0dfb

fix(MenuController): 등록된 주문이 없을 경우에 대한 예외 처리 추가, Command Enum 적용, 메서드 분리

bc1d88c

refactor: domain 패키지 내 세부 패키지 분리

88c57ef

Ridealist changed the title ~~[치킨 POST 미션] 고준보 미션 제출합니다.~~ [치킨 POS 미션] 고준보 미션 제출합니다. Dec 12, 2022

Ridealist added 7 commits December 12, 2022 23:04

test(ApplicationTest): 어플리케이션 Integration Test 구현

5038b0a

test(Application): 주문 최대 가능 수량 초과 에러 테스트 추가

26f335f

test(Repository): 번호 기준 데이터 가져오는 메서드 테스트 구현

dd357eb

test(Order): 최대 주문 가능 수량 초과 에러 테스트 구현

995f009

test(OrderRepository): 쿼리 메소드들에 대한 단위 테스트 구현

45f1c37

test(Pay): 결제 관련 메서드 단위 테스트 구현

4f9b56a

test(PayMethod): 결제 수단 유효성 검증, 결제 수단에 따른 할인률 단위 테스트 구현

7b574dd

khj1 reviewed Dec 13, 2022

View reviewed changes

gitchannn reviewed Dec 13, 2022

View reviewed changes

		.filter(order -> order.getTable().equals(table))
		.filter(order -> order.getMenu().equals(menu))

		Table table = TableRepository.findTableByNumber(1);
		Menu menu = MenuRepository.findMenuByNumber(1);

[치킨 POS 미션] 고준보 미션 제출합니다. #2

Are you sure you want to change the base?

[치킨 POS 미션] 고준보 미션 제출합니다. #2

Uh oh!

Conversation

Ridealist commented Dec 12, 2022

Uh oh!

khj1 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants