[라테] 치킨집 미션 제출합니다. #6
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- MenuOrder 삭
- 테이블에서 오더 기능을 처리하는 기능 추가 - 테이블 주문 내역을 계산할 때 할인 정책을 통해 할인 가격을 계산하는 기능 추
- Payment에 따른 할인 정책 추가
- Exception 발생 시에도 프로그램이 동작하도록 수정 - OutputView 중 잘못된 줄바꿈 수정 - 주문 목록이 계산 후에도 비어있지 않은 점 수정
hongbin-dev
reviewed
Jun 27, 2020
Comment on lines
+28
to
+36
| outputView.printTables(tableOrderService.findAllTables()); | ||
| int tableNumber = inputView.inputTableNumber(); | ||
|
|
||
| outputView.printMenus(menuService.findAllMenus()); | ||
| int menuNumber = inputView.inputMenuNumber(); | ||
|
|
||
| int amount = inputView.inputAmount(); | ||
|
|
||
| tableOrderService.addOrder(new TableOrderRequest(tableNumber, menuNumber, amount)); |
There was a problem hiding this comment.
잘못입력하면 바로 에러가 발생하는게 아니라 addOrder에서 터지겠네요..?
사용자 입장에서는 열심히 입력했는데 슬플것같아요ㅠㅠ
There was a problem hiding this comment.
콘솔을 웹처럼 구현하다보니 불가피한 부분이 존재하더라고요...
최대한 도메인을 서비스 안에 있게하고 싶었습니다. ㅠ
Comment on lines
14
to
22
| public boolean runIfNotExit() { | ||
| try { | ||
| Command command = Command.findByNumber(inputView.inputCommandNumber()); | ||
| return !command.execute(chickenController); | ||
| } catch (Exception e) { | ||
| System.out.println(e.getMessage()); | ||
| return true; | ||
| } | ||
| } |
src/main/java/domain/Payment.java
Outdated
Comment on lines
18
to
21
| return Arrays.stream(values()) | ||
| .filter(payment -> payment.number == number) | ||
| .findAny() | ||
| .orElseThrow(() -> new IllegalArgumentException("해당하는 결제 수단이 존재하지 않습니다.")); |
There was a problem hiding this comment.
Suggested change
| return Arrays.stream(values()) | |
| .filter(payment -> payment.number == number) | |
| .findAny() | |
| .orElseThrow(() -> new IllegalArgumentException("해당하는 결제 수단이 존재하지 않습니다.")); | |
| return Arrays.stream(values()) | |
| .filter(payment -> payment.number == number) | |
| .findAny() | |
| .orElseThrow(() -> new IllegalArgumentException("해당하는 결제 수단이 존재하지 않습니다. number=" + number)); |
상세한 exception을 남겨볼까요?
Comment on lines
25
to
27
| public static Amount sum(Amount first, Amount second) { | ||
| return new Amount(first.amount + second.amount); | ||
| } |
There was a problem hiding this comment.
정적팩터리메서드를 만드셨군요! 싱글인스턴스로 만드는건 어떨까요?
There was a problem hiding this comment.
그죠 99까지만 있으면 되니깐 인스턴스를 반환하게끔 하면 되겠네요. 수정했습니다.
Comment on lines
29
to
31
| public Amount add(int other) { | ||
| return new Amount(amount + other); | ||
| } |
Comment on lines
+49
to
+52
| return menuOrders.keySet().stream() | ||
| .filter(menu -> menu.isSameCategory(category)) | ||
| .map(menuOrders::get) | ||
| .reduce(0, (subTotal, amount) -> amount.calculateSum(subTotal), Integer::sum); |
There was a problem hiding this comment.
comparable을 구현해서 stream.sum()을 하는건 어떨까요?
|
|
||
| public class Table { | ||
| private final int number; | ||
| private final Order order = Order.empty(); |
Comment on lines
25
to
28
| Table table = findTableByNumber(number); | ||
| if (table.isOrderEmpty()) { | ||
| throw new IllegalArgumentException("주문이 비어 있습니다."); | ||
| } |
kimevanjunseok
requested changes
Jun 28, 2020
Comment on lines
+28
to
+36
| outputView.printTables(tableOrderService.findAllTables()); | ||
| int tableNumber = inputView.inputTableNumber(); | ||
|
|
||
| outputView.printMenus(menuService.findAllMenus()); | ||
| int menuNumber = inputView.inputMenuNumber(); | ||
|
|
||
| int amount = inputView.inputAmount(); | ||
|
|
||
| tableOrderService.addOrder(new TableOrderRequest(tableNumber, menuNumber, amount)); |
| public boolean runIfNotExit() { | ||
| try { | ||
| Command command = Command.findByNumber(inputView.inputCommandNumber()); | ||
| return !command.execute(chickenController); |
There was a problem hiding this comment.
execute가 boolean값을 리턴하니까 어색한데....라테는 어떤가요?
|
|
||
| final int tableNumber = InputView.inputTableNumber(); | ||
| ChickenController chickenController = new ChickenController(inputView, |
| return category == other; | ||
| } | ||
|
|
||
| public int calculateMultiplePrice(Amount amount) { |
There was a problem hiding this comment.
앨런에게도 물어본 이야기입니다.
#7 (comment)
라테는 어떻게 생각하나요?
There was a problem hiding this comment.
저는 요구사항 중 getter를 쓰지 않는다.를 지키려다 보니 이런 코드가 나왔네요.
사실 여기서는 감싸여진 클래스가 많다보니 이런 왔다갔다(?)하는 코드가 있는 거라고 생각해서, 이 경우는 getter를 쓰는게 좋을 수도 있을 것 같습니다. 다만 이번 단계에서는 getter 사용 금지에 초점을 맞춰 진행해보았습니다.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
잘 부탁드립니다.
웹처럼 만들려고 하다보니 오잉? 하는 부분이 있으실 겁니다 ㅠ