[크리스마스 프로모션] 전호건 리뷰부탁드립니다. by Arachneee · Pull Request #1 · Arachneee/java-christmas-6-Arachneee

Arachneee · 2023-11-16T06:03:30Z

📌 주요 포인트

할인 이벤트와 선물 이벤트를 인터페이스와 추상 클래스로 분리와 동시에 공통 로직 통합
여러 종류의 이벤트를 Enum과 함수형 인터페이스로 관리
MVC 패턴에 Service, Repository를 추가하여 역할 분리
Enum과 EnumMap으로 불필요 클래스 제거 및 통합 관리
계층형 Dto를 활용한 View와 Domain 의존성 분리

📊 UML

날짜 범위를 검증하여 예외를 던지도록 구현

크리스마스 할인 계산을 위한 날짜 차이 계산 기능 추가

entry의 stream을 이용하여 총 주문 가격 계산

10000원 미만일 경우 할인 금액이 0원을 반환하도록 설계, Menu enum을 사용하여 메뉴의 카테고리화 적용

추상클래스 템플릿을 이용하여 적용 불가능 케이스 0원 반환

Category로 조회가 필요하여 enum 타입 분리

나머지를 이용하여 주말 확인

Menu 항목을 찾는 성능 향상을 위해 제목 Map을 추가했습니다.

Event 인터페이스의 static 메소드를 활용하여 중복 로직을 제거하면서 EventService의 중복 로직까지 제거했습니다.

chunghyeon-kim

Enum이 효과적으로 사용되었고 클래스들의 책임이 깔끔하게 분리되어 있다고 느꼈습니다. 제가 처음 본 자바 API도 많이 사용되었고, 제가 생각하지 못한 세세한 부분들까지 잘 구현되어 있어서 개선점을 찾기보다는 감탄하고 많이 배워갑니다.

chunghyeon-kim · 2023-11-16T07:06:35Z

src/main/java/christmas/domain/order/constant/Menu.java

+import java.util.Map;
+
+public enum Menu {
+


카테고리 enum을 포함하는 Menu라는 enum을 만드는 방법이 있었네요. 잘봤습니다

저도 카테고리는 그냥 String형으로 만들었는데 카테고리 enum을 포함하는 방식이 훨씬 좋아보이네요 하나 배우고갑니다! 👍

chunghyeon-kim · 2023-11-16T07:19:50Z

src/main/java/christmas/domain/event/Badge.java

+        this.minPrice = minPrice;
+    }
+
+    public static Badge from(final int totalDiscount) {


minPrice 내림차순으로 정렬하고 findFirst를 이용해서 혜택금액에 따른 배지 수여의 책임을 Enum클래스 자체가 가지도록 하셨네요.
감탄했습니다!

내림차순을 통해 뱃지의 열거 순서와 상관없이 정확한 값을 계산할 수 있네요 :)

minPrice를 내림차순으로 정렬했을 때에만 from 메서드가 정상 작동하지 않을까..? 했는데
.sorted()를 통해 한 차례 정렬 후, 값을 찾네요!
꼼꼼한 처리에 대해 배워갑니다!👍

chunghyeon-kim · 2023-11-16T07:31:16Z

src/main/java/christmas/controller/converter/OrderConverter.java

+
+public final class OrderConverter {
+
+    private static final Pattern MENU_COUNT_PATTERN = compile("^[가-힣A-Za-z]+-\\d+(?:,[가-힣A-Za-z]+-\\d+)*$");


Pattern클래스의 compile메서드와 정규식을 이용한 필터링방법 잘봤습니다.
깔끔하고 명확하네요!

Pattern을 미리 컴파일해두셔서 매번 컴파일하지 않아 성능 향상이 되겠네요 👍

compile을 사용하는 방법이 있네요..! 좋은 방법 배워갑니다! ☺️

chunghyeon-kim · 2023-11-16T07:47:10Z

src/main/java/christmas/controller/converter/OrderConverter.java

컨버터 클래스를 만들어서 View에서 받은 input을 적절하게 변환하는 책임을 부여하셨군요. 잘봤습니다

chunghyeon-kim · 2023-11-16T07:59:02Z

src/main/java/christmas/domain/order/constant/DayOfWeek.java

+        return findDayOfWeekByMod(mod);
+    }
+
+    private DayOfWeek findDayOfWeekByMod(final int mod) {


요일 enum을 만들고 어떤 요일인지 구하는 기능을 해당 enum 클래스에 구현하셨네요. 잘봤습니다

pricelees

정말 잘 읽고 갑니다!

pricelees · 2023-11-16T07:42:30Z

src/main/java/christmas/controller/converter/OrderConverter.java

+    private static final Pattern MENU_COUNT_PATTERN = compile("^[가-힣A-Za-z]+-\\d+(?:,[가-힣A-Za-z]+-\\d+)*$");
+    private static final String MENU_SPLIT_SIGNAL = ",";
+    private static final String COUNT_SPLIT_SIGNAL = "-";
+    private static final String INVALID_START = "0";
+    private static final int TITLE_INDEX = 0;
+    private static final int COUNT_INDEX = 1;


split에 들어가는 상수 네이밍이 너무 좋은 것 같아요!

pricelees · 2023-11-16T07:49:29Z

src/main/java/christmas/controller/converter/OrderConverter.java

+    private static int convertToInt(final String input, final ErrorMessage errorMessage) {
+        validateStartZero(input, errorMessage);
+
+        try {
+            return Integer.parseInt(input);
+        } catch (NumberFormatException numberFormatException) {
+            throw OrderException.from(errorMessage);
+        }
+    }
+
+    private static void validateStartZero(final String input, final ErrorMessage errorMessage) {
+        if (input.startsWith(INVALID_START)) {
+            throw OrderException.from(errorMessage);
+        }
+    }


여기서 ErrorMessage는 enum이니 바로 사용해도 될 것 같은데, 메서드의 입력값으로 지정하신 이유가 있을까요??

문자를 숫자로 변환하는 과정에서 날짜인 경우와 메뉴의 수량인 경우를 구분하기 위해서 ErrorMessage를 전달했습니다. 같은 로직인데 메소드를 2개로 나누는 것보다 합치는 것이 좋다고 생각해서 입력값으로 전달했습니다. 더 좋은 방법이 있을지 궁금하네요.

pricelees · 2023-11-16T07:49:36Z

src/main/java/christmas/controller/converter/OrderConverter.java

+    private static int convertToInt(final String input, final ErrorMessage errorMessage) {
+        validateStartZero(input, errorMessage);
+
+        try {
+            return Integer.parseInt(input);
+        } catch (NumberFormatException numberFormatException) {
+            throw OrderException.from(errorMessage);
+        }
+    }
+
+    private static void validateStartZero(final String input, final ErrorMessage errorMessage) {
+        if (input.startsWith(INVALID_START)) {
+            throw OrderException.from(errorMessage);
+        }
+    }


여기서 ErrorMessage는 enum이니 바로 사용해도 될 것 같은데, 메서드의 입력값으로 지정하신 이유가 있을까요??

pricelees · 2023-11-16T07:55:15Z

src/main/java/christmas/domain/event/Badge.java

+    public static Badge from(final int totalDiscount) {
+        return Arrays.stream(values())
+                .sorted(Comparator.comparing(Badge::getMinPrice).reversed())
+                .filter(badge -> badge.isSatisfiedCondition(totalDiscount))
+                .findFirst()
+                .orElse(NONE);
+    }


이 부분만 떼서 읽으면, sorted 부분이 어쩌면 "가격이 높은 순서대로 정렬"로 해석될 여지도 있는 것 같습니다. minPrice 라는 변수명보단, 조금 더 "뱃지를 얻기위한 최소금액"이라는 것을 나타내는 변수명이면 좋을 것 같네요!

pricelees · 2023-11-16T08:02:07Z

src/main/java/christmas/domain/event/discount/DiscountEventType.java

+    SPECIAL_DISCOUNT("특별 할인", order -> order.isNotSunDay() && order.isNotChristmasDay(),
+            order -> Amount.SPECIAL_ONCE.value);


isNotSunDay()와 isNotChristmasDay()를 합친 메서드를 하나 더 정의하면, 형식면에서 더 통일되어 보일 것 같습니다!

isNotStarDay() 메소드를 만들었었는데 'order 객체가 starday가 뭔지 알아야할 필요가 있을까?' 생각했습니다. 그래서 분리했습니다.

pricelees · 2023-11-16T08:38:52Z

src/main/java/christmas/domain/order/constant/Week.java

+public enum Week {
+    WEEKDAY(DESSERT),
+    WEEKEND(MAIN);


Week이라는 이름으로부터, DESSERT, MAIN에 대한 정보를 생각하기엔 힘들 것 같습니다! 이 카테고리를 할인한다는 의도가 좀 더 드러나면 좋을 것 같네요!

pricelees · 2023-11-16T08:39:12Z

src/main/java/christmas/exception/OrderException.java

예외 클래스까지 하나하나 정의하셨네요.. 감탄하고 갑니다 정말

이번 미션 요구사항에서 아래와 같은 내용이 있었는데요,

사용자가 잘못된 값을 입력할 경우 IllegalArgumentException를 발생시키고, "[ERROR]"로 시작하는 에러 메시지를 출력 후 그 부분부터 입력을 다시 받는다.
Exception이 아닌 IllegalArgumentException, IllegalStateException 등과 같은 명확한 유형을 처리한다.

IllegalArgumentException을 발생시키라고 했는데 그 아래 줄에는 " ~~ 등과 같은 명확한 유형" 이라는 말이 붙어서.. 사용자 예외를 사용해도 되는지 많이 헷갈리더라구요
호건님께서는 어떻게 생각하시나요?

저도 좀 헷갈렸는데 Exception이 아닌 IllegalArgumentException, IllegalStateException 라는 표현이IllegalArgumentException 보다 상위 클래스를 사용하지 말라는 말로 해석했습니다. 사용자 예외는 자식 클래스이므로 사용해도 괜찮을 것 같습니다.

pricelees · 2023-11-16T08:41:22Z

src/main/java/christmas/response/EventDetailResponse.java

빌더 패턴의 실사용은 프리코스를 진행하면서 처음 보는 것 같은데.. 값의 설정에 있어 굉장히 효율적이겠다는 생각이 드네요.. 메모 하고 갑니다..

builder패턴 많이 배우고 갑니다..!
OutputView에서 EventDetailResponse의 함수를 하나하나 호출해서 출력하는데,
view만 볼때에는 "한번에 묶어서 넘겨주면 좋지않을까?" 라고 생각했는데 eventDetailResponse를 읽어보니 OutputView에서 출력 포맷을 한번에 묶어주는 것도 정말 괜찮은 방법인 것 같습니다!

매개변수에 final이 빠졌네요!
또, Builder 패턴은 매개변수가 많을 때 정말 좋은 패턴이지만,
record를 사용했음에도 중복하여 사용하는 것은 오히려 코드의 복잡도를 높이는 것이 아닌가 싶습니다!
이 부분에 대해 호건님의 의견이 궁금하네요 :)

빌더에 final를 확인하지 못했네요. record에도 builder를 사용한 것은 괜찮다고 생각합니다. record에 setter와 builder가 없어 생성자로 생성할 경우 순서를 기억하기 쉽지 않다고 생각해서 빌더패턴을 사용했습니다.

pricelees · 2023-11-16T08:46:53Z

src/test/java/christmas/controller/OrderControllerTest.java

+@DisplayName("주문 컨트롤러는")
+class OrderControllerTest {


클래스에도 DisplayName을 지정할수 있는지는 이제 알았네요!

pricelees · 2023-11-16T08:48:12Z

src/test/java/christmas/controller/OrderControllerTest.java

+        writer = new TestWriter();
+        reader = new TestQueueReader();


테스트에 쓰이는 클래스를 별도로 지정해도 문제가 되지 않나요? 저도 테스트에 공통적으로 쓰이는 상수 클래스를 하나 정의하긴 했는데.. 궁금해서 여쭙습니다..

Mock처럼 Test 객체를 만들어서 사용해봤습니다.

jisu-om

전반적으로 추상화를 잘 이용하신 것 같아요 :) 4주동안 고생 많으셨습니다!
repository 사용, Response의 사용 잘 배워갑니다~!
그리고 위에 UML 이미지,,, 알록달록 넘 예쁘네요.. 혹시 직접 만드신 걸까요?? 탐나요...ㅎㅎㅎ

제 코드리뷰도 부탁드려요 :)
jisu-om/java-christmas6-jisu-om#3

jisu-om · 2023-11-16T08:20:11Z

src/main/java/christmas/controller/OrderController.java

+        return getByRoof(() -> Order.of(day, OrderConverter.convertToMenu(inputView.readMenuAndCount())));
+    }
+
+    private <T> T getByRoof(final Supplier<T> method) {


요렇게 함수형 인터페이스 적용하는 것 깔끔합니다!! 👍

이 부분은 재사용성을 고려하여 따로 Util로 빼는 것은 어떨까요?

빼는 것도 좋아보입니다.

jisu-om · 2023-11-16T08:27:38Z

src/main/java/christmas/controller/converter/OrderConverter.java

+
+public final class OrderConverter {
+
+    private static final Pattern MENU_COUNT_PATTERN = compile("^[가-힣A-Za-z]+-\\d+(?:,[가-힣A-Za-z]+-\\d+)*$");


요런 방식도 있었네요!! 잘 떨어지네요 고생하셨습니다!

오와 패턴을 이용한 형식 검증 너무 좋은데요. 좋은 코드 감사합니다 👍

jisu-om · 2023-11-16T08:35:31Z

src/main/java/christmas/view/OutputView.java

+    private enum Response {
+        HELLO("안녕하세요! 우테코 식당 12월 이벤트 플래너입니다."),
+        PREVIEW_EVENT("12월 %d일에 우테코 식당에서 받을 이벤트 혜택 미리 보기!"),
+        MENU_COUNT("%s %d개"),
+        DISCOUNT_RESULT("%s: -%,d원"),
+        POSITIVE_MONEY("%,d원"),
+        NEGATIVE_MONEY("-%,d원"),
+        NONE("없음"),
+        ENTER(lineSeparator());
+
+        private final String value;
+
+        Response(final String value) {
+            this.value = value;
+        }
+    }
+
+    private enum Header {
+        MENU("<주문 메뉴>"),
+        BEFORE_TOTAL_PRICE("<할인 전 총주문 금액>"),
+        GIFT("<증정 메뉴>"),
+        DISCOUNT("<혜택 내역>"),
+        TOTAL_DISCOUNT("<총혜택 금액>"),
+        AFTER_PAYMENT("<할인 후 예상 결제 금액>"),
+        BADGE("<12월 이벤트 배지>");
+
+        private final String value;
+
+        Header(final String value) {
+            this.value = value;
+        }
+
+        public String getValueWithEnter() {
+            return Response.ENTER.value + value;
+        }
+    }


이렇게 private enum 클래스로 메세지 관리를 해도 되겠네요!

내부 enum으로 메시지를 관리하니까 훨씬 깔끔하네요! 👍

jisu-om · 2023-11-16T08:36:39Z

src/main/java/christmas/response/OrderSummaryResponse.java

@@ -0,0 +1,9 @@
+package christmas.response;
+
+public record OrderSummaryResponse(OrderResponse orderResponse, EventDetailResponse eventDetailResponse) {


record 로 OrderResponse 와 EventDetailResponse 를 묶어서 관리하는 것 좋은 것 같아요!

jisu-om · 2023-11-16T08:42:58Z

src/main/java/christmas/response/EventDetailResponse.java

+        String badge
+) {
+
+    public static class EventDetailResponseBuilder {


저도 ResultDto(view에 넘겨주는 정보 역할) 에 builder 클래스를 만들었다가 지우고 필요한 도메인을 파라미터로 받아서 생성 메서드 안에서 처리하자 싶었어요.

EventDetailService 의 buildEventDetailResponse 메서드를 보면
서비스 레이어니까 이렇게 생성 로직을 드러내도 괜찮은걸까요?
저도 요렇게 했다가 너무 다 드러내는 것 같아서 생성 로직을 static factory 메서드인 ResultDto.of() 안에 숨겼거든요

private EventDetailResponse buildEventDetailResponse( final int priceBeforeDiscount, final int totalBenefitsAmount, final int totalDiscountBenefits ) { return EventDetailResponseBuilder.builder() .priceBeforeEvent(priceBeforeDiscount) .giftMenuResponses(giftService.getGiftMenuResponse()) .activeEvents(getAllActiveEvent()) .totalBenefitsAmount(totalBenefitsAmount) .priceAfterEvent(calculatePriceAfterDiscount(priceBeforeDiscount, totalDiscountBenefits)) .badge(getBadgeTitle(totalBenefitsAmount)) .build(); }

view 출력을 위한 정보 전달 과정이 궁금했는데 잘 배우고 갑니다 :)

jisu-om · 2023-11-16T08:47:58Z

src/main/java/christmas/domain/event/discount/DiscountEventType.java

+    private enum Amount {
+        CHRISTMAS_ONCE(100),
+        CHRISTMAS_OFFSET(1_000),
+        WEEKDAY_ONCE(2_023),
+        WEEKEND_ONCE(2_023),
+        SPECIAL_ONCE(1_000),
+        BASE_MIN(10_000),
+        ZERO(0);
+
+        private final int value;
+
+        Amount(final int value) {
+            this.value = value;
+        }
+    }


이렇게 한 곳에서 관리하는 것 편리해보여요!
다만 ZERO 이렇게 상수 이름을 짓기보다 NONE 이렇게 네이밍 하는 게 좋다고 다른 코드리뷰에서 들었어요 :)
(예를 들면 ',' 를 COMMA 로 이름짓는게 아니라 DELIMITER 라고 이름 붙이기)

jisu-om · 2023-11-16T08:51:58Z

src/main/java/christmas/domain/event/discount/DiscountRepository.java

+
+import christmas.domain.event.EventRepository;
+
+public class DiscountRepository extends EventRepository<DiscountEventType> {


Repository를 적용할 수 있겠네요 Repository까지 필요할까 싶어서 코드 타고타고 들어갔는데 EventRepository 의 메서드들이 유용하네요 :)
상속을 잘 사용하니까 편한 것 같아요!

우와 꼭 JPA Repository를 보는 것 같네요..
상속을 이렇게까지 사용할 수 있구나.. 감탄하고 배워갑니다..!😲

twoosky

4주차 미션 수행 하시느라 고생 많으셨습니다. 제네릭을 적극 활용하신 것이 인상 깊었어요. 코드 잘 봤습니다 👍

twoosky · 2023-11-16T09:32:16Z

src/main/java/christmas/controller/converter/OrderConverter.java

+
+public final class OrderConverter {
+
+    private static final Pattern MENU_COUNT_PATTERN = compile("^[가-힣A-Za-z]+-\\d+(?:,[가-힣A-Za-z]+-\\d+)*$");


오와 패턴을 이용한 형식 검증 너무 좋은데요. 좋은 코드 감사합니다 👍

twoosky · 2023-11-16T10:37:08Z

src/main/java/christmas/controller/converter/OrderConverter.java

+                        () -> new EnumMap<>(Menu.class)));
+    }
+
+    private static <T> BinaryOperator<T> throwingDuplicateMenuException() {


오 이런 방법도 있는지 처음 알았네요! toMap에 다양한 시그니처가 있군요 공부해봐야겠네요 좋은 코드 감사합니다 ㅎ

twoosky · 2023-11-16T10:42:14Z

src/main/java/christmas/domain/event/Event.java

+
+    String getTitle();
+
+    static <T extends Enum<T> & Event> EnumMap<T, Integer> applyAll(final Class<T> eventType, final Order order) {


디스코드 포스팅 제목처럼 제네릭을 정말 잘 활용하셨네요. Enum을 제네릭으로 사용하는 방법 알아갑니다 🏃

twoosky · 2023-11-16T10:56:48Z

src/main/java/christmas/domain/event/discount/DiscountEventType.java

+        return title;
+    }
+
+    private enum Amount {


Amount값을 DiscountEventType에서 갖고 있지 않고 enum으로 분리하신 이유가 궁금해요

내부 enum으로 갖고 있는 형태입니다. 다만 상수가 많아 private enum으로 관리했습니다.

twoosky · 2023-11-16T10:59:19Z

src/main/java/christmas/domain/event/gift/GiftRepository.java

+import java.util.Map;
+import java.util.Map.Entry;
+
+public class GiftRepository extends EventRepository<GiftEventType> {


오 EventRepository로 공통된 코드를 분리한게 인상 깊네요. 👍

twoosky · 2023-11-16T11:02:09Z

src/main/java/christmas/domain/event/gift/GiftRepository.java

+    public Map<String, Integer> getActiveMenuCounts() {
+        return eventAmounts.entrySet().stream()
+                .filter(this::isActive)
+                .collect(toMap(this::getMenuTitle, this::getMenuCount));


불변 보장을 위해 Collections.unmodifiableMap()을 사용하는 것은 어떤가요?

그 부분을 깜빡했네요. 감사합니다!

twoosky · 2023-11-16T11:09:03Z

src/main/java/christmas/domain/order/constant/DayOfWeek.java

+        return findDayOfWeekByMod(mod);
+    }
+
+    private DayOfWeek findDayOfWeekByMod(final int mod) {


자바의 LocalDate를 활용하면 날짜에 해당하는 요일을 바로 구할 수 있어요. 한번 찾아보시면 좋을거 같습니다 :)

헉..!! 저도 처음 알았네요..!! 자바의 LocalDate..!! 코멘트 보면서 배워갑니다!

twoosky · 2023-11-16T11:13:12Z

src/main/java/christmas/response/EventResponse.java

+
+public record EventResponse(String title, int amount) {
+
+    public static EventResponse of(final String title, final int amount) {


record를 생성하면 각 필드에 argument가 있는 public constructor가 생성되는 것으로 알고 있어요. 근데 생성자를 사용하지 않고, 정적 메서드를 정의하신 이유가 궁금해요

다른 클래스들과 통일성을 위해서 정적 메소드를 사용했는데 생성자도 괜찮겠네요.

mingeun2154

제가 관심있었던 이벤트의 추상화, 방문 날짜 위주로 리뷰했습니다.
할인 뿐만 아니라 증정 이벤트까지 하나의 인터페이스로 표현 하신 점, 함수형 인터페이스를 사용하여 이벤트별 혜택 금액 계산식을 동적으로 설정한 점이 인상적이였습니다.
제가 하고싶었지만 하지 못했던 것들입니다.
December, Week, DaysOfWeek 등의 클래스들은 java.time API를 사용하면 굳이 만들지 않았어도 될 것 같습니다.
4주간 코드 리뷰를 하면서 한 손가락에 꼽을 정도로 많은 것을 배울 수 있었습니다.

mingeun2154 · 2023-11-16T10:11:29Z

src/main/java/christmas/domain/event/discount/DiscountEventType.java

+    CHRISTMAS_D_DAY_DISCOUNT("크리스마스 디데이 할인", Order::isAfterChristmas,
+            order -> order.countDayAfterFirstDay() * Amount.CHRISTMAS_ONCE.value + Amount.CHRISTMAS_OFFSET.value),
+    WEEKDAY_DISCOUNT("평일 할인", Order::isWeekend,
+            order -> order.countWeekEventMenu() * Amount.WEEKDAY_ONCE.value),
+    WEEKEND_DISCOUNT("주말 할인", Order::isWeekday,
+            order -> order.countWeekEventMenu() * Amount.WEEKEND_ONCE.value),
+    SPECIAL_DISCOUNT("특별 할인", order -> order.isNotSunDay() && order.isNotChristmasDay(),
+            order -> Amount.SPECIAL_ONCE.value);
+
+    private final String title;
+    private final Predicate<Order> unavailable;
+    private final Function<Order, Integer> calculate;


와 이 방법은 생각 못 했네요!
저는 할인 행사를 최대한 하나의 클래스로 표현하려고 노력했지만 이벤트 별로 혜택 금액 계산 로직을 통일하지 못해 두 가지 클래스로 나누어 표현한 것이 아쉬웠습니다.
혜택 금액 계산식을 함수형 인터페이스로 전달해서 이벤트 종류와 혜택 금액 계산 로직을 분리하여 다양한 이벤트를 하나의 클래스로 표현하는 방법은 전혀 생각하지 못했습니다. 😳
덕분에 함수형 인터페이스에 대한 시야가 트였습니다

mingeun2154 · 2023-11-16T10:17:46Z

src/main/java/christmas/domain/event/EventRepository.java

+public abstract class EventRepository<T extends Enum<T> & Event> {
+
+    private static final int ZERO = 0;
+    protected EnumMap<T, Integer> eventAmounts;


다양한 이벤트를 하나의 enum으로 표현하고 그에 대한 일급 컬렉션까지 구현하셨네요.
제가 하고 싶었지만 도달하지 못한 것들입니다. 있어야 할 것들이 당연하다는 듯이 다 있다는 느낌을 받았습니다.

ZERO는 0이라는 값을 담고 있을 뿐인데 상수화 하신 이유가 무엇인가요?

또, int형 ZERO라는 상수가 다양한 곳에서 상수로 정의되어 있습니다!
전역으로 사용하는 데이터라면 각 클래스에 두는 것보다 한 곳에 모아 관리하는 것이 추후 유지보수에 더욱 유리하지 않을까요? 🤗🤗

다른 부분들에 final을 잘 달아주신만큼, eventAmounts가 final이 아닌 이유에 대해서 궁금합니다!
실수로 eventAmounts 객체가 바뀌는 문제를 방지하기 위해 이 부분에도 final을 달아주면 더욱 좋겠습니다 :)

이 클래스를 추상 클래스로 유지해야 하기에 final을 못다셨다면,
EnumMap을 굳이 외부로부터 주입받아야 하는 이유가 있을까요?
이 Repository의 저장 Map을 결정하는 것은 외부가 아닌 Repository의 책임으로 두는 것이 SRP를 지키는 방법이라 생각합니다.

init보다 정적 팩토리 메소드를 활용해서 Repository를 생성하는 방식으로 하는 것이 더 좋았을 것 같습니다.

mingeun2154 · 2023-11-16T11:29:57Z

src/main/java/christmas/domain/event/discount/DiscountEventType.java

+
+    CHRISTMAS_D_DAY_DISCOUNT("크리스마스 디데이 할인", Order::isAfterChristmas,
+            order -> order.countDayAfterFirstDay() * Amount.CHRISTMAS_ONCE.value + Amount.CHRISTMAS_OFFSET.value),
+    WEEKDAY_DISCOUNT("평일 할인", Order::isWeekend,
+            order -> order.countWeekEventMenu() * Amount.WEEKDAY_ONCE.value),
+    WEEKEND_DISCOUNT("주말 할인", Order::isWeekday,
+            order -> order.countWeekEventMenu() * Amount.WEEKEND_ONCE.value),
+    SPECIAL_DISCOUNT("특별 할인", order -> order.isNotSunDay() && order.isNotChristmasDay(),
+            order -> Amount.SPECIAL_ONCE.value);
+
+    private final String title;
+    private final Predicate<Order> unavailable;
+    private final Function<Order, Integer> calculate;


이벤트 별로 혜택 금액을 계산하는 방식이 달라 하나의 클래스로 표현하지 못했는데 이런 방법이 있었네요 😳
혜택금액 계산식을 고정된 식이 아니라 함수형 인터페이스로 넘겨주고 런타임에 주문 정보를 넘겨 받는 방법은 생각지도 못했습니다

mingeun2154 · 2023-11-16T11:42:32Z

src/main/java/christmas/domain/event/EventRepository.java

+import java.util.Map.Entry;
+
+
+public abstract class EventRepository<T extends Enum<T> & Event> {


모든 종류의 이벤트를 담을 수 있는 일급 컬렉션까지... 있어야 할 것들이 다 있다는 느낌을 받았습니다
저도 이렇게 모든 이벤트를 하나의 컬렉션에 담아서 관리하고싶었지만 하지 못했습니다

mingeun2154 · 2023-11-16T11:43:53Z

src/main/java/christmas/domain/order/constant/DayOfWeek.java

+import christmas.domain.order.Day;
+import java.util.Arrays;
+
+public enum DayOfWeek {


DayOfWeek는 이미 java.time 패키지에 있는 클래스인데 새로 정의한 이유가 궁금합니다

있는지 몰랐네요.ㅎㅎ

mingeun2154 · 2023-11-16T11:45:42Z

src/main/java/christmas/domain/order/constant/December.java

+import static christmas.domain.order.constant.DayOfWeek.MONDAY;
+import static christmas.domain.order.constant.DayOfWeek.SUNDAY;
+
+public enum December {


java.time 패키지에 년, 월, 일에 대한 API가 있는데 따로 December 클래스를 구현하신 이유가 궁금합니다

LocalDate를 사용하지 않아서 따로 정의했습니다.

oyoungsun

리뷰 남기고 갑니다!
코드를 보면서, 또 코드에 담긴 의도를 파악하며 정말 유익한 시간 보냈습니다
어쩌다 보니 리뷰에 궁금증이나 의견 나누고 싶은 것에 대해 더 많이 쓴것 같지만...🤣
시간 여유가 되신다면 답글 기다리겠습니다 ㅎ

oyoungsun · 2023-11-17T08:40:43Z

src/main/java/christmas/view/InputView.java

+            this.value = value;
+        }
+    }
+}


close()를 통해 controller가 view를 닫을 수 있게 하는것, 클래스 내부 상수를 enum으로 정리하는것 정말 크게 배우고 갑니다!

이렇게 내부에서 정의하면 외부에서 호출하는 것보다 가독성도 올라가고, 무엇보다 클래스 내부의 enum필드끼리는 연관성이 있다고도 생각되네요 👍

oyoungsun · 2023-11-17T08:47:22Z

src/main/java/christmas/response/EventDetailResponse.java

builder패턴 많이 배우고 갑니다..!
OutputView에서 EventDetailResponse의 함수를 하나하나 호출해서 출력하는데,
view만 볼때에는 "한번에 묶어서 넘겨주면 좋지않을까?" 라고 생각했는데 eventDetailResponse를 읽어보니 OutputView에서 출력 포맷을 한번에 묶어주는 것도 정말 괜찮은 방법인 것 같습니다!

oyoungsun · 2023-11-17T08:49:28Z

src/main/java/christmas/domain/event/EventRepository.java

+    private String getEventTitle(final Entry<T, Integer> entry) {
+        return entry.getKey().getTitle();
+    }
+}


repository 로 도메인의 기본적인 비지니스 로직까지 분리하셨다니 정말 감탄이 나옵니다..!! 리뷰를 할수록 점점 저만 더 배워가는 기분이네요 💦💦💦

oyoungsun · 2023-11-17T08:51:32Z

src/main/java/christmas/controller/converter/OrderConverter.java

+        } catch (NumberFormatException numberFormatException) {
+            throw OrderException.from(errorMessage);
+        }
+    }


Int 에 대해서도 Pattern으로 들어온 input이 모두 숫자로만 구성되어있는지 확인하는 방법도 있지 않을까요? (그러나 long 이 들어오는 경우를 생각하면 역시.. try-catch로 사용하는 방법밖에 없을까요? 호건님의 의견이 궁금합니다!

저의 경우에는 코드를 짜다보니 계속 try-catch를 최대한 분리하고 싶어지더라구요.. 💦

정규식으로도 할 수 있는데 정규식이 너무 복잡해지거나 Interger의 범위를 넘을 수 있는 경우 때문에 try catch로 했습니다

oyoungsun · 2023-11-17T08:54:52Z

src/main/java/christmas/service/order/OrderService.java

+
+        return eventDetailService.getEventDetail(priceBeforeDiscount);
+    }
+}


이부분도 호건님의 의견이 궁금해 남겨봅니다!
예전에 spring관련 프로젝트를 하며, controller -> service-> domain순으로 하위 계층을 호출하고, 같은 계층(컨트롤러가 컨트롤러를 호출하는)의 경우에는 지양하는 것이 좋다 라는 의견이 나왔는데, 이번 코드리뷰를 하면서도 service가 service를 호출하는 것을 지양하는 의견이 나왔답니다 😁 이부분에 대해서는 어떻게 생각하시나요?

OrderService 가 eventDetailService를 호출해서, controller의 run()이 가벼워졌지만 오히려 controller가 해야할 일을 service가 담당하게 되지는 않을까요?

Controller끼리의 호출은 지양하는 것이 맞다고 생각해요. 하지만 Service의 경우 순환 참조가 되지 않게 호출하는 방법은 좋다고 생각합니다.
Controller에서 여러 Service를 호출할 경우 여러 Service의 트랜잭션을 구현할 수 없게됩니다. 또한 Controller끼리의 호출도 트랜젝션을 구현할 수 없기 때문에 좋지 않은 설계라고 생각하고요.
반면에 Service의 경우 계층형으로 Service가 호출되면 트랜젝션 전파로 인하여 트랜젝션을 보장 할 수 있습니다. 또한 클래스를 분리하면서 다른 곳에서 사용할 수 도 있게 되어 더 확장성이 높은 설계라고 생각합니다.
같은 레이어끼리의 호출을 지양하는 이유는 트랜젝션 문제와 순환참조문제가 있기 때문이라고 생각합니다.

답변 감사합니다! 좀더 찾아보다보니 또 비슷한 토론 타래가 있어 함께 공유해봅니다 😁
말씀대로 생길 수 있는 문제는 트랜젝션, 순환 참조의 문제가 있는 것 같습니다.
다만 실제 서비스 상황까지 확장해본다면 상위 서비스->하위 서비스로의 의존도 고려해볼 수 있을 것같네요!
woowacourse/retrospective#15
https://jangjjolkit.tistory.com/62

Arachneee

좋은 리뷰 감사합니다!

Arachneee · 2023-11-17T10:33:59Z

src/main/java/christmas/service/order/OrderService.java

+
+        return eventDetailService.getEventDetail(priceBeforeDiscount);
+    }
+}


Controller끼리의 호출은 지양하는 것이 맞다고 생각해요. 하지만 Service의 경우 순환 참조가 되지 않게 호출하는 방법은 좋다고 생각합니다.
Controller에서 여러 Service를 호출할 경우 여러 Service의 트랜잭션을 구현할 수 없게됩니다. 또한 Controller끼리의 호출도 트랜젝션을 구현할 수 없기 때문에 좋지 않은 설계라고 생각하고요.
반면에 Service의 경우 계층형으로 Service가 호출되면 트랜젝션 전파로 인하여 트랜젝션을 보장 할 수 있습니다. 또한 클래스를 분리하면서 다른 곳에서 사용할 수 도 있게 되어 더 확장성이 높은 설계라고 생각합니다.
같은 레이어끼리의 호출을 지양하는 이유는 트랜젝션 문제와 순환참조문제가 있기 때문이라고 생각합니다.

inpink

안녕하세요 호건님!
프리코스 고생 많으셨습니다 :)

inpink · 2023-11-19T15:11:08Z

src/main/java/christmas/service/event/EventDetailService.java

+
+    private String getBadgeTitle(final int totalBenefitsAmount) {
+        return Badge.from(totalBenefitsAmount).getTitle();
+    }


Badge는 매번 Order객체가 EventDetailService를 통해 계산해야만 알 수 있는 것으로 보입니다!
이번 미션 중 "배지는 2024 새해 이벤트에서 활용할 예정입니다."라는 요구사항이 있습니다.

Badge 데이터가 어딘가에 저장되지 않는다면,
지금 발생한 Order에 의해, 지금 판단된 Badge 데이터가 추후에도 같은 값을 가지지 않을 위험성이 있지 않을까요?
추후 이 로직이 변경이라도 된다면 데이터의 무결성을 지키지 못하는 문제도 생기네요!

또, Order에 대해 발생한 Badge 데이터는 몇 번을 계산해도 동일한데, 사용할 때마다 매번 계산해줄 필요가 있을까요? 불필요한 연산 과정이라 생각합니다!

나중에 데이터를 다시 사용할 것을 생각하면 따로 저장해 두는 것이 좋겠네요. 제가 출력에만 초점을 맞춘 설계를 한 것 같네요.

inpink · 2023-11-19T15:13:38Z

src/main/java/christmas/service/event/EventDetailService.java

+        giftService.applyEventAll(GiftEventType.class, order);
+    }
+
+    public EventDetailResponse getEventDetail(final int priceBeforeDiscount) {


모든 곳에서 원시값 int을 그대로 사용하신 것으로 보입니다!
추후 돈의 단위가 int가 아닌 long, double 등으로 변경된다면 int라고 사용해준 모든 부분을 수정해줘야겠네요!
객체지향 생활 체조 원칙에 따르면, 모든 원시값을 포장하라 는 원칙이 있습니다.
이 부분을 공부해보시면 좋을 것 같아요 :)

돈도 원시값 포장하는 것에 동의합니다. 그럼 각 이벤트 해택내역, 총 결제내역, 총 이득내역 등을 전부 같은 원시값으로 포장하는 것이 좋을까요? 각각이 의미하는 것은 다르지만 돈이라는 공통 속성이므로 같은 원시값으로 포장하는 것이 맞을지 궁금합니다.

이 부분은 요구 사항에 따라 달라질 수 있는 판단이라 생각해요 :)

저는 이번 4주차 미션에서는 Money라는 하나의 공통 객체만 두었습니다!

하지만 3주차 미션에서는 구매 금액, 우승 상금을 따로 객체로 두는 것이 더 좋다고 생각합니다!
구매 금액, 우승 상금은 돈이라는 공통점이 있기에 필요하다면 Money로 추상화를 하는 것도 좋겠네요!

3주차 -> 구매 금액은 1000의 배수여야 함. 우승 상금은 그런 요구 사항이 없음.
4주차 -> 각 금액과 관련된 차별화된 개별 요구사항이 없음.

이런 차이점이 있네요!

inpink · 2023-11-19T15:14:47Z

src/main/java/christmas/controller/OrderController.java

+        this.inputView = inputView;
+        this.outputView = outputView;
+        this.orderService = orderService;
+    }


final을 꼼꼼히 달아주셨네요 👍 이를 통해 예기치 않은 변화로 발생하는 문제를 방지할 수 있겠네요 :)

inpink · 2023-11-19T15:15:37Z

src/main/java/christmas/controller/OrderController.java

+        return getByRoof(() -> Order.of(day, OrderConverter.convertToMenu(inputView.readMenuAndCount())));
+    }
+
+    private <T> T getByRoof(final Supplier<T> method) {


이 부분은 재사용성을 고려하여 따로 Util로 빼는 것은 어떨까요?

inpink · 2023-11-19T15:16:15Z

src/main/java/christmas/controller/converter/OrderConverter.java

+
+public final class OrderConverter {
+
+    private static final Pattern MENU_COUNT_PATTERN = compile("^[가-힣A-Za-z]+-\\d+(?:,[가-힣A-Za-z]+-\\d+)*$");


Pattern을 미리 컴파일해두셔서 매번 컴파일하지 않아 성능 향상이 되겠네요 👍

inpink · 2023-11-19T15:34:11Z

src/main/java/christmas/domain/event/Badge.java

+        this.minPrice = minPrice;
+    }
+
+    public static Badge from(final int totalDiscount) {


내림차순을 통해 뱃지의 열거 순서와 상관없이 정확한 값을 계산할 수 있네요 :)

inpink · 2023-11-19T15:50:09Z

src/main/java/christmas/domain/order/Order.java

+    private static final int TOTAL_COUNT_MAX = 20;
+    private static final int ONCE_COUNT_MIN = 1;
+    private final Day day;
+    private final Map<Menu, Integer> menuCount;


이 부분은 일급컬렉션화 하면 어떨까요?
Order이 Map<Menu, Integer> menuCount가 해야 할 역할까지 담당하고 있어 복잡도가 높아 보입니다!
Order의 코드가 길어진 것은 위의 문제가 있기 때문이라 생각해요!

Map<Menu, Integer> menuCount와 관련된 내용은 MenuCount 객체가 하도록 분리하여 SRP를 지킬 수 있겠네요 :)

이 부분도 일급컬렉션을 사용하는 것이 좋았겠네요

inpink · 2023-11-19T15:52:46Z

src/main/java/christmas/domain/order/Order.java

+        validate(menuCount);
+
+        return new Order(day, menuCount);
+    }


정적 팩터리 메서드를 잘 사용하고 계신 점이 좋네요 :)
검증 내용은 이곳이 아닌 Order 생성자 내부에 두는 것이 더 좋지 않을까요?
검증을 정적 팩터리 메서드로 둠으로 불필요한 static이 반복되는 것으로 보입니다!

static이 반복되는 문제는 신경쓰지 못했네요. 생성자 내부로 두는 것 좋은 방법인 것 같습니다.

inpink · 2023-11-19T15:54:45Z

src/main/java/christmas/exception/OrderException.java

이번 미션 요구사항에서 아래와 같은 내용이 있었는데요,

사용자가 잘못된 값을 입력할 경우 IllegalArgumentException를 발생시키고, "[ERROR]"로 시작하는 에러 메시지를 출력 후 그 부분부터 입력을 다시 받는다.
Exception이 아닌 IllegalArgumentException, IllegalStateException 등과 같은 명확한 유형을 처리한다.

IllegalArgumentException을 발생시키라고 했는데 그 아래 줄에는 " ~~ 등과 같은 명확한 유형" 이라는 말이 붙어서.. 사용자 예외를 사용해도 되는지 많이 헷갈리더라구요
호건님께서는 어떻게 생각하시나요?

inpink · 2023-11-19T15:58:56Z

src/main/java/christmas/response/EventDetailResponse.java

매개변수에 final이 빠졌네요!
또, Builder 패턴은 매개변수가 많을 때 정말 좋은 패턴이지만,
record를 사용했음에도 중복하여 사용하는 것은 오히려 코드의 복잡도를 높이는 것이 아닌가 싶습니다!
이 부분에 대해 호건님의 의견이 궁금하네요 :)

Arachneee

생각해보지 못한 부분 리뷰해주셔서 감사합니다.

Arachneee · 2023-11-20T02:31:34Z

src/main/java/christmas/controller/OrderController.java

+        return getByRoof(() -> Order.of(day, OrderConverter.convertToMenu(inputView.readMenuAndCount())));
+    }
+
+    private <T> T getByRoof(final Supplier<T> method) {


빼는 것도 좋아보입니다.

Arachneee · 2023-11-20T02:37:24Z

src/main/java/christmas/domain/order/Order.java

+        validate(menuCount);
+
+        return new Order(day, menuCount);
+    }


static이 반복되는 문제는 신경쓰지 못했네요. 생성자 내부로 두는 것 좋은 방법인 것 같습니다.

Arachneee · 2023-11-20T02:39:51Z

src/main/java/christmas/exception/OrderException.java

저도 좀 헷갈렸는데 Exception이 아닌 IllegalArgumentException, IllegalStateException 라는 표현이IllegalArgumentException 보다 상위 클래스를 사용하지 말라는 말로 해석했습니다. 사용자 예외는 자식 클래스이므로 사용해도 괜찮을 것 같습니다.

Arachneee · 2023-11-20T03:01:29Z

src/main/java/christmas/response/EventDetailResponse.java

빌더에 final를 확인하지 못했네요. record에도 builder를 사용한 것은 괜찮다고 생각합니다. record에 setter와 builder가 없어 생성자로 생성할 경우 순서를 기억하기 쉽지 않다고 생각해서 빌더패턴을 사용했습니다.

Arachneee · 2023-11-20T03:08:01Z

src/main/java/christmas/domain/event/EventRepository.java

+
+    public void init(final EnumMap<T, Integer> eventAmounts) {
+        this.eventAmounts = eventAmounts;
+    }


초기화 -> 계산 방식으로 동작하게 설계했는데 초기화와 동시에 계산하는 방식으로 설계하는 것이 더 좋았을 거 같습니다.

Arachneee · 2023-11-20T03:11:57Z

src/main/java/christmas/domain/event/EventRepository.java

+public abstract class EventRepository<T extends Enum<T> & Event> {
+
+    private static final int ZERO = 0;
+    protected EnumMap<T, Integer> eventAmounts;


init보다 정적 팩토리 메소드를 활용해서 Repository를 생성하는 방식으로 하는 것이 더 좋았을 것 같습니다.

Arachneee · 2023-11-20T03:12:25Z

src/main/java/christmas/domain/event/EventRepository.java

+
+    public int calculateTotal() {
+        return eventAmounts.values().stream().mapToInt(Integer::intValue).sum();
+    }


그 부분을 생각하지 못했네요

SeongUk52

레이어가 굉장히 체계적이고 배울게 많은 코드였습니다. 👍
이정도는 해야 우테코 합격하겠다는 생각이 드네요
4주 동안 정말 고생 많으셨습니다! 🙌

SeongUk52 · 2023-11-20T08:14:34Z

src/main/java/christmas/Application.java

+import static christmas.config.PlannerConfig.orderController;
+
 public class Application {
+
    public static void main(String[] args) {
-        // TODO: 프로그램 구현
+        orderController().run();


확장성을 고려해서 Config파일로 orderController를 생성하셨군요 이런 방법이 있는 줄 몰랐네요
좋은 지식 배우고갑니다! 👍

SeongUk52 · 2023-11-20T08:25:35Z

src/main/java/christmas/domain/order/Day.java

+    public boolean isChristmasDay() {
+        return December.isChristMas(day);
+    }
+
+    public boolean isSunDay() {
+        return DayOfWeek.from(this).isSunDay();
+    }
+
+    public boolean isWeekend() {
+        return DayOfWeek.from(this).isWeekend();
+    }
+


LocalDate를 사용하지 않고 모든 로직을 직접 구현하셨군요 🥶
저는 LocalDate로 충분하다고 생각했었는데 아니었나봅니다..😱
직접 구현하신 이유가 듣고싶어요! 🥹

변수가 날짜만 있어서 직접구현했습니다. 확장성을 고려한다면 LocalDate를 사용하는 것이 더 편할 것 같습니다.

SeongUk52 · 2023-11-20T08:27:28Z

src/main/java/christmas/domain/order/constant/Menu.java

+import java.util.Map;
+
+public enum Menu {
+


저도 카테고리는 그냥 String형으로 만들었는데 카테고리 enum을 포함하는 방식이 훨씬 좋아보이네요 하나 배우고갑니다! 👍

SeongUk52 · 2023-11-20T08:29:51Z

src/main/java/christmas/view/OutputView.java

+    private enum Response {
+        HELLO("안녕하세요! 우테코 식당 12월 이벤트 플래너입니다."),
+        PREVIEW_EVENT("12월 %d일에 우테코 식당에서 받을 이벤트 혜택 미리 보기!"),
+        MENU_COUNT("%s %d개"),
+        DISCOUNT_RESULT("%s: -%,d원"),
+        POSITIVE_MONEY("%,d원"),
+        NEGATIVE_MONEY("-%,d원"),
+        NONE("없음"),
+        ENTER(lineSeparator());
+
+        private final String value;
+
+        Response(final String value) {
+            this.value = value;
+        }
+    }
+
+    private enum Header {
+        MENU("<주문 메뉴>"),
+        BEFORE_TOTAL_PRICE("<할인 전 총주문 금액>"),
+        GIFT("<증정 메뉴>"),
+        DISCOUNT("<혜택 내역>"),
+        TOTAL_DISCOUNT("<총혜택 금액>"),
+        AFTER_PAYMENT("<할인 후 예상 결제 금액>"),
+        BADGE("<12월 이벤트 배지>");
+
+        private final String value;
+
+        Header(final String value) {
+            this.value = value;
+        }
+
+        public String getValueWithEnter() {
+            return Response.ENTER.value + value;
+        }
+    }


내부 enum으로 메시지를 관리하니까 훨씬 깔끔하네요! 👍

hey-sung · 2023-11-21T04:54:26Z

src/main/java/christmas/config/PlannerConfig.java

+
+    public static OrderController orderController() {
+        return new OrderController(inputView(), outputView(), orderService());
+    }


우왓.. Config 파일에서 이런 식으로 객체를 생성하는 방법을 생각지도 못했는데, static 메서드를 사용하니 깔끔하네요..! 👍

hey-sung · 2023-11-21T04:58:11Z

src/main/java/christmas/controller/converter/OrderConverter.java

+
+public final class OrderConverter {
+
+    private static final Pattern MENU_COUNT_PATTERN = compile("^[가-힣A-Za-z]+-\\d+(?:,[가-힣A-Za-z]+-\\d+)*$");


compile을 사용하는 방법이 있네요..! 좋은 방법 배워갑니다! ☺️

hey-sung · 2023-11-21T05:00:15Z

src/main/java/christmas/domain/event/Badge.java

+        this.minPrice = minPrice;
+    }
+
+    public static Badge from(final int totalDiscount) {


minPrice를 내림차순으로 정렬했을 때에만 from 메서드가 정상 작동하지 않을까..? 했는데
.sorted()를 통해 한 차례 정렬 후, 값을 찾네요!
꼼꼼한 처리에 대해 배워갑니다!👍

hey-sung · 2023-11-21T05:06:41Z

src/main/java/christmas/domain/event/discount/DiscountEventType.java

+
+    private final String title;
+    private final Predicate<Order> unavailable;
+    private final Function<Order, Integer> calculate;


함수형 인터페이스에 대해서 알고는 있었지만 막상 어떻게 적용해야 할지 모르는 경우가 허다 했는데,
호건님 코드를 보면서 이렇게 사용할 수 있구나..!! 라는 생각이 많이 드네요..
정말 많이 배워갑니다..! 👍

hey-sung · 2023-11-21T05:07:43Z

src/main/java/christmas/domain/event/discount/DiscountRepository.java

+
+import christmas.domain.event.EventRepository;
+
+public class DiscountRepository extends EventRepository<DiscountEventType> {


우와 꼭 JPA Repository를 보는 것 같네요..
상속을 이렇게까지 사용할 수 있구나.. 감탄하고 배워갑니다..!😲

hey-sung · 2023-11-21T05:08:54Z

src/main/java/christmas/domain/order/Day.java

+    private Day(final int day) {
+        this.day = day;
+    }
+
+    public static Day from(final int day) {
+        validate(day);
+        return new Day(day);
+    }


저도 이 부분이 궁금합니다!
생성자에 넣으면 validate와 관련된 메서드들을 private static이 아닌, private로 설정할 수 있을 것 같아요

hey-sung · 2023-11-21T05:10:18Z

src/main/java/christmas/domain/order/constant/DayOfWeek.java

+        return findDayOfWeekByMod(mod);
+    }
+
+    private DayOfWeek findDayOfWeekByMod(final int mod) {


헉..!! 저도 처음 알았네요..!! 자바의 LocalDate..!! 코멘트 보면서 배워갑니다!

hey-sung · 2023-11-21T05:12:37Z

src/main/java/christmas/domain/order/constant/Menu.java

+    public static Menu from(final String title) {
+        return menus.computeIfAbsent(title, key -> {
+            throw OrderException.from(INVALID_ORDER);
+        });
+    }


computeIfAbsent라는 좋은 메서드가 있었네요..!
저는 stream을 통해 존재 여부를 확인했는데 훨씬 깔끔하고 간단하네요!

Arachneee added 30 commits November 9, 2023 15:49

docs: 핵심 기능 및 구현 기능 목록 정리

6ce312b

test: 크리스마스 할인 계산 테스트 작성

2ab22a7

test: 주문 날짜 생성 테스트 작성

5148d1c

feat: 주문 날짜 생성 기능 구현

dd073f1

날짜 범위를 검증하여 예외를 던지도록 구현

docs: 주문 날짜 생성 기능 구현

e2c41f0

날짜 범위를 검증하여 예외를 던지도록 구현

test: 주문 날짜 차이 계산 테스트

0343767

feat: 주문 날짜 차이 계산 기능

7446c16

크리스마스 할인 계산을 위한 날짜 차이 계산 기능 추가

docs: 주문 날짜 차이 계산 기능

2557ff1

크리스마스 할인 계산을 위한 날짜 차이 계산 기능 추가

test: 주문의 총 금액이 10000원 이상이어야 할인 적용가능

fe502fa

test: 총 주문 가격 계산 테스트

6cc70b8

feat: 총 주문 가격 계산 기능 추가

f7bf2f5

entry의 stream을 이용하여 총 주문 가격 계산

docs: 총 주문 가격 계산 기능 추가

eaf977a

entry의 stream을 이용하여 총 주문 가격 계산

feat: 총 금액 10000원 이상 검증 기능 구현

6941daa

10000원 미만일 경우 할인 금액이 0원을 반환하도록 설계, Menu enum을 사용하여 메뉴의 카테고리화 적용

test: 주문 날짜가 25일을 넘어가면 할인 적용이 안된다.

3f34870

feat: 크리스마스 할인 적용 기능 구현

679a011

추상클래스 템플릿을 이용하여 적용 불가능 케이스 0원 반환

test: 평일에 디저트 메뉴 1개당 2023원 할인 합계를 계산할 수 있다.

c7c3bcd

feat: 평일 할인 계산 기능 추가

724850d

Category로 조회가 필요하여 enum 타입 분리

docs: 평일 할인 계산 기능 추가

0e5a39d

Category로 조회가 필요하여 enum 타입 분리

test: 평일에만 할인이 적용된다.

e1dd93d

test: 주문일자가 주말인지 평일일지 확인할 수 있다.

4ad1a77

feat: 주문일자가 주말인지 확인기능 추가

7ace7f7

나머지를 이용하여 주말 확인

docs: 주문일자가 주말인지 확인기능 추가

331faa7

나머지를 이용하여 주말 확인

feat: 평일에만 할인 적용 기능 추가

9369b1a

test: 날짜의 나머지를 계산할 수 있다.

03ae550

test: 주말 계산 기능 위치 이동

b6aa4b4

refactor: WEEK enum으로 평일/주말 계산 기능 분리

46e266c

test: display 이름 수정

a6bed6f

style: static import 추가

7363139

test: 주말 할인 적용 기능 추가

9f563ca

feat: 주말 할인 적용 기능 추가

bdff0dc

Arachneee added 18 commits November 14, 2023 02:06

test: final 추가

930c064

chore: 패키지 변경

7bd2adf

docs: ReadMe 수정

8255fac

refactor: Menu에 static Map 추가

5a93576

Menu 항목을 찾는 성능 향상을 위해 제목 Map을 추가했습니다.

test: Nested 추가

e93de6e

docs: 아이콘 수정

e2904b7

test: 날짜 0으로 시작 문자 입력 예외 테스트 추가

bf4100e

style: static import추가, final 추가

55d15cd

docs: readme 수정

ff7936d

docs: 오타 수정

a76f1be

test: 오타 수정

cec2816

rename: 메소드 인자명 수정

e5ea3f4

docs: UML 수정

48e68f5

docs: 깨지는 이모티콘 변경

2e7aabb

docs: 깨지는 이모티콘 변경

307c23a

refactor: DiscountEventType 과 GiftEventType 의 중복 static 메소드 제거

47436ea

Event 인터페이스의 static 메소드를 활용하여 중복 로직을 제거하면서 EventService의 중복 로직까지 제거했습니다.

test: DiscountEventType 과 GiftEventType 의 중복 static 메소드 제거

638d9d6

Event 인터페이스의 static 메소드를 활용하여 중복 로직을 제거하면서 EventService의 중복 로직까지 제거했습니다.

docs: 주요포인트 순서 변경

4b589e5

chunghyeon-kim reviewed Nov 16, 2023

View reviewed changes

pricelees reviewed Nov 16, 2023

View reviewed changes

jisu-om reviewed Nov 16, 2023

View reviewed changes

Arachneee mentioned this pull request Nov 16, 2023

[크리스마스 프로모션] 양홍주 미션 제출합니다. inpink/java-christmas-6-inpink#2

Open

twoosky reviewed Nov 16, 2023

View reviewed changes

mingeun2154 reviewed Nov 16, 2023

View reviewed changes

oyoungsun reviewed Nov 17, 2023

View reviewed changes

Arachneee commented Nov 17, 2023

View reviewed changes

inpink reviewed Nov 19, 2023

View reviewed changes

Arachneee commented Nov 20, 2023

View reviewed changes

SeongUk52 reviewed Nov 20, 2023

View reviewed changes

hey-sung reviewed Nov 21, 2023

View reviewed changes


		public final class OrderConverter {

		private static final Pattern MENU_COUNT_PATTERN = compile("^[가-힣A-Za-z]+-\\d+(?:,[가-힣A-Za-z]+-\\d+)*$");

		SPECIAL_DISCOUNT("특별 할인", order -> order.isNotSunDay() && order.isNotChristmasDay(),
		order -> Amount.SPECIAL_ONCE.value);

		@DisplayName("주문 컨트롤러는")
		class OrderControllerTest {

		@@ -0,0 +1,9 @@
		package christmas.response;

		public record OrderSummaryResponse(OrderResponse orderResponse, EventDetailResponse eventDetailResponse) {


		import christmas.domain.event.EventRepository;

		public class DiscountRepository extends EventRepository<DiscountEventType> {


		String getTitle();

		static <T extends Enum<T> & Event> EnumMap<T, Integer> applyAll(final Class<T> eventType, final Order order) {


		public record EventResponse(String title, int amount) {

		public static EventResponse of(final String title, final int amount) {

		import java.util.Map.Entry;


		public abstract class EventRepository<T extends Enum<T> & Event> {

Conversation

Arachneee commented Nov 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chunghyeon-kim 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!

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!

pricelees 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!

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!

jisu-om left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Arachneee commented Nov 16, 2023 •

edited

Loading

jisu-om left a comment •

edited

Loading