[크리스마스이벤트] 박진현 리뷰용 PR #3
Open
Conversation
This file contains hidden or 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
이름, 카테고리, 금액을 저장
크리스마스 디데이 , 평일, 주말, 특별할인
Dompoo
reviewed
Nov 27, 2024
Comment on lines
+20
to
+22
| if (orderItems.stream().anyMatch(item -> item.getMenu().getCategory().equals("음료") && orderItems.size() == 1)) { | ||
| throw new IllegalArgumentException("[ERROR] 음료만 주문할 수 없습니다."); | ||
| } |
There was a problem hiding this comment.
음료를 1개만 주문했을 때는 잘 걸릴 것 같은데, 음료만 2개 주문하면 통과될 것 같아요!
| CHAMPAGNE("샴페인", "음료", BigDecimal.valueOf(25_000)); | ||
|
|
||
| private final String name; | ||
| private final String category; |
There was a problem hiding this comment.
카테고리는 분류가 정해져있으니까, Enum으로 관리하면 더 좋을 것 같아요!
Comment on lines
+10
to
+15
| public BigDecimal applyDiscount(Order order, int date) { | ||
| if (date >= 1 && date <= 25) { | ||
| BigDecimal discountAmount = BigDecimal.valueOf(1000 + (date - 1) * 100); | ||
| return discountAmount.min(order.getTotalPriceBeforeDiscount()); | ||
| } | ||
| return BigDecimal.ZERO; |
There was a problem hiding this comment.
어디서 봤는지 기억은 안나는데, 이런 if 구조에서
1~25일 사이인게 메인이잖아요? 그러면 그 외의 것을 if문 안에 넣어서 탈출시키는 방식으로 주요 코드를 indent 1에 계속 두는 것이 더 읽기 좋은 코드를 짠다고 들은 적이 있어요.
if (date < 1 || date > 25) {
return BigDecimal.ZERO;
}
BigDecimal discountAmount = BigDecimal.valueOf(1000 + (date - 1) * 100);
return discountAmount.min(order.getTotalPriceBeforeDiscount());그래야 if문이 많이 중첩되는 경우에도 흐름을 따라가기 쉽다고 하더라고요!
Comment on lines
+8
to
+15
| private static final int GIFT_THRESHOLD = 120_000; | ||
|
|
||
| public Menu checkGiftEligibility(Order order) { | ||
| if (order.getTotalPriceBeforeDiscount().compareTo(new java.math.BigDecimal(GIFT_THRESHOLD)) >= 0) { | ||
| return Menu.CHAMPAGNE; | ||
| } | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Gift도 DiscountPolicy처럼 관리했으면 더 좋았을 것 같아요!
지금은 너무 하드코딩 되어서 나중에 다른 Gift를 추가하고 싶어도 고칠 것이 많아 보입니닷.
Comment on lines
+40
to
+56
| private Badge assignBadge(BigDecimal totalDiscount, Menu gift) { | ||
| BigDecimal totalBenefit = totalDiscount; | ||
| if (gift != null) { | ||
| totalBenefit = totalBenefit.add(gift.getPrice()); | ||
| } | ||
|
|
||
| if (totalBenefit.compareTo(new BigDecimal("20000")) >= 0) { | ||
| return Badge.SANTA; | ||
| } | ||
| if (totalBenefit.compareTo(new BigDecimal("10000")) >= 0) { | ||
| return Badge.TREE; | ||
| } | ||
| if (totalBenefit.compareTo(new BigDecimal("5000")) >= 0) { | ||
| return Badge.STAR; | ||
| } | ||
| return Badge.NONE; | ||
| } |
There was a problem hiding this comment.
요 로직은 Badge의 내부 구현이라고 생각해서, Badge가 직접 처리해도 좋을 것 같은데, 어떻게 생각하시나요?
Comment on lines
+7
to
+15
| public String readDate() { | ||
| System.out.println("안녕하세요! 우테코 식당 12월 이벤트 플래너입니다."); | ||
| System.out.println("12월 중 식당 예상 방문 날짜는 언제인가요? (숫자만 입력해 주세요!)"); | ||
| return Console.readLine(); | ||
| } | ||
| public String readOrder() { | ||
| System.out.println("주문하실 메뉴와 개수를 입력해 주세요. (e.g. 해산물파스타-2,레드와인-1,초코케이크-1)"); | ||
| return Console.readLine(); | ||
| } |
There was a problem hiding this comment.
요구사항을 잘 보시면, 1월에도 이벤트를 계속 진행할 것 처럼 적혀있어요.
12월로 하드코딩하지 않고, 값을 외부에서 주입받도록 하는 것이 좋아 보입니다!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
No description provided.