[우테코 6기 프리코스] 4주차 피드백 정리

별의개발자커비·2023년 11월 16일
0

우테코 도전기

목록 보기
35/37

개요

프리코스의 마지막 4주차 미션! 비록 프리코스는 끝났지만 그렇다고 성장을 멈출수는 없지😉 이번주에 달린 리뷰도 모두 정리해보자!

📌 String.fomatted: 예외메시지

에러 메시지도 스트링의 formatted를 사용하면 되겠구나!

📌 Objects.isNull: String으로 받아도 null 검증!

그동안 String으로 받아서 null 검증이 불필요하거나,
파라미터가 String이라 null을 주입하면 오류가 발생하곤 했는데
이 방식을 이용하면 되겠다!

📌 놓친 예외 검증

split 후 요소 갯수가 2개인지 검증을 안해서
양송이스프-2-2 이런 입력이 통과되게 되었다🥲

📌 enum안에 abstract을 넣는 방식!

스터디원 현준님의 코드에서 발견한 부분인데 재미있는 방식이라 스크랩 해두고 싶었다!

package christmas.domain.benefit;

import christmas.domain.user.User;
import christmas.domain.menu.Menu;
import christmas.domain.user.order.OrderDetail;

public enum Event {
    CHRISTMAS_D_DAY_EVENT("크리스마스 디데이 할인", 1000) {
        @Override
        public boolean isApplicable(User user) {
            return user.isVisitChristmasDday();
        }

        @Override
        public int calculateDiscount(User user) {
            return CHRISTMAS_D_DAY_EVENT.discountPrice + (user.getVisitDate() - 1) * 100;
        }
    },
    WEEKDAY_EVENT("평일 할인", 2023) {
        @Override
        public boolean isApplicable(User user) {
            return user.isVisitWeekday();
        }

        @Override
        public int calculateDiscount(User user) {
            int dessertOrderCount = user.getOrderDetails().stream()
                    .filter(orderDetail -> Menu.isDessert(orderDetail.getMenuName()))
                    .mapToInt(OrderDetail::getQuantity)
                    .sum();
            return WEEKDAY_EVENT.discountPrice * dessertOrderCount;
        }
    },
    WEEKEND_EVENT("주말 할인", 2023) {
        @Override
        public boolean isApplicable(User user) {
            return !user.isVisitWeekday();
        }

        @Override
        public int calculateDiscount(User user) {
            int mainOrderCount = user.getOrderDetails().stream()
                    .filter(orderDetail -> Menu.isMain(orderDetail.getMenuName()))
                    .mapToInt(OrderDetail::getQuantity)
                    .sum();
            return WEEKEND_EVENT.discountPrice * mainOrderCount;
        }
    },
    SPECIAL_DAY_EVENT("특별 할인", 1000) {
        @Override
        public boolean isApplicable(User user) {
            return user.isVisitSpecialDay();
        }

        @Override
        public int calculateDiscount(User user) {
            return SPECIAL_DAY_EVENT.discountPrice;
        }
    },
    GIFT_EVENT("증정 이벤트", 1) {
        @Override
        public boolean isApplicable(User user) {
            return Gift.givable(user.getTotalOrderPrice());
        }

        @Override
        public int calculateDiscount(User user) {
            int giftPrice = Gift.findPriceByTotalOrderPrice(user.getTotalOrderPrice());
            return GIFT_EVENT.discountPrice * giftPrice;
        }
    };

    private final String name;
    private final int discountPrice;

    Event(String name, int discountPrice) {
        this.name = name;
        this.discountPrice = discountPrice;
    }

    public String getName() {
        return this.name;
    }

    public abstract boolean isApplicable(User user);

    public abstract int calculateDiscount(User user);
}

📌 매니저라는 네이밍🚫

여러 객체가 도출되는 종합 객체에서 manager 라는 네이밍도 좋은 것 같다!.. 고 생각했는데

( + 추가)

📌 map 테스트: containsExactlyAnyOrderEntries

Map을 key, value를 쌍로 테스트 할 수 있는 assertJ 메소드 GET!

📌 enum의 선언 순서 의존 해결!

📌 출력에 필요없는 값? 하지만 도메인에는 있어야지!

할인이 없는 항목을 Map에 저장하지 않으면 출력시에 별다른 작업 없이 할인 있는 것만 보여줄 수 있겠군!
이렇게 생각을 하고 구현을 바꿨던 부분인데,
리뷰를 보고 생각해보니 '구현의 간단함'을 위해 '필요한 값의 저장'을 하지 않았다는 생각이 든다!

값의 저장을 출력에 초점을 맞추고 있었다는 걸 인식하게 되었달까..
출력이 목적이 되기보다는 값이 도메인에 저장되기에 유의미한 값인지를 생각해보기!

📌 리스코프 치환 원칙 (LSP)

부모 객체와 자식 객체가 있을 때 부모 객체를 호출하는 동작에서 자식 객체가 부모 객체를 완전히 대체할 수 있다는 원칙
약간 DIP와 비슷한 느낌을 받았고 map을 사용할 때도 선언을 더 상위 객체인 Map으로 해주고 생성시 구현부만 필요한 hasMap이나 enumMap으로 하는 것이 더 좋을 것 같다!

해당 리뷰에서 겪어보아도 좋을 것 같다고 하신 것 얼마 있지 않아 깨달았다...
처음에 enumMap으로 사용하다 리팩토링하면서 LinkedHashMap으로 바꿨더니 저 enumMap이 걸쳐있는 모든 메소드를 다 바꿔줘야했다ㅎㅎ
이래서 상위 객체로 선언해줘야하는 구나 직접 깨달음!

이펙티브 자바 아이템 64 '객체는 인터페이스를 사용해 참조하라'에서 발췌한 부분을 함께 첨부하자면

  1. 프로그램이 훨씬 유연해질 수 있다.
    나중에 구현 클래스를 교체하고자한다면 enumMap이 선언된 모든 곳을 교체하지 않고 그저 새 클래스의 생성자를 호출해주기만 하면 된다. (= 처음 선언된 그 부분만 enumMap을 예를 들어 hasMap으로 바꿔주면 된다.)
  2. 단, 원래의 클래ㅡ가 큭별한 기능을 제공해, 주변 코드가 이 기능에 기대어 동작한다면 새로운 클래스도 반드시 같은 기능을 제공해야한다.
    예를들어, linkedHashSet을 첫번째 선언으로 했는데 이를 hashSet으로 바꾸면 문제가 된다. hashSet은 반복자의 순회 순서를 보장하지 않기 때문이다.

📌 finalAmount 관련

처음에는 FinalAmount 객체를 만들었다가 전체에서 할인 금액을 빼는 간단한 로직 하나만 처리하는데 굳이 객체로 만들 필요가 있을까? 해당 연산만 하는 메소드만 뽑아 쓰면 더 간단하지 않을까? 하고 해당 방식으로 구현했었다.

하지만 리뷰를 보다보니, 객체를 생성해 처리하는지 아니면 정적 메소드로 처리하는지 출력 결과는 같지만 도메인 로직의 중요도를 고려해야한다는 점을 (뼈를 맞으며...🦴) 다시 한 번 인지하게 되었다.

📌 ==와 equals는 다르다

생각해보니, == 와 equals의 차이를 크게 생각하지 않았던 것 같다.
덕분에 객체의 내용을 비교하는 equals객체지향적 의미에 더 적절하겠다는 생각을 하게 되었다!

📌 날짜 하드코딩하지 않는 방법 - localDate, dayOfWeek

이번주 날짜와 관련된 부분을 나는 당당하게 하드코딩했다!😂
하지만 다른 분들의 코드를 보니 LocalDate를 사용해 간단하게 구현하셨더라..! 바로 커비 들어간닷

getDayOfWeek

LocalDate 객체를 통해 그 날짜가 어느 요일인지 추출할 수도 있다!
요일은 DayOfWeek 객체로 추출되어 내장 요일 상수을 그대로 사용하면 된다!

getDayOfMonth

LocalDate 객체들을 갖고 getDayOfMonth 메소드를 쓰면 몇 일에 해당하는 숫자만 추출 가능해서 날짜의 크기를 비교할 수도 있다!

📌 재입력의 컨트롤러 책임?

물론 '입력'과 관련한 책임 측면에서는 리뷰처럼 view쪽의 책임으로 볼 수도 있을 것 같다고 생각한다.

다만, 프로그램의 특정 부분에서 예외 상황이 발생할 때 이를 처리하고 상호작용한다는 측면에서 컨트롤러가 해당 책임을 가질 수 있다고 생각했던 것 같다!
따라서, 예외 처리나 재입력은 view에서 컨트롤러까지 여러 층에서 책임이 공유될 수 있다는 생각으로 해당 방식으로 구현한 것 같다!

📌 이벤트 구현 방식 변경: 확장성 고려

이 부분에 대해 고민이 많았는데 다른 분들의 코드를 보고
이벤트별로 공통적으로 갖는 타입별 할인 금액을 도출하는 applyDiscount메소드를 중심으로 Interface를 상속받게 하면 되겠구나! 했다.

📌 메뉴의 enum vs 메소드 방식

처음에는 메뉴를 왜 추가할 수 없지? 그냥 enum에 추가할 메뉴 상수를 한 줄 추가하면 되는 거 아닌가?하고 생각을 했는데,

enum의 정체성을 생각해보니,

  1. 컴파일 시점에 상수로 정의되어 있어, 실행 중에 새로운 항목을 추가하는 것이 안되겠다.
  2. Enum은 정적인 상수들을 나열하는데에 사용되기 때문에 코드를 변경하지 않고는 새로운 Enum 상수를 추가하기 어렵다.
  3. 따라서, Enum으로 구현된 Menu는 고정된 항목을 정의하는 데 사용되지만, 새로운 항목을 추가할 수 없는 한계가 있다는 것!

유연한 설계의 측면에서는,

MenuBoardInitializer로 각 메뉴를 List로 정의하고 초기화하는 방식이 있는데. 이 방식은 새로운 메뉴를 추가하기 위해 해당 메서드에 새로운 메뉴를 추가하는 것만으로 새로운 기능을 구현할 수 있다. 이것은 기존 코드를 변경하지 않고도 새로운 메뉴를 추가할 수 있는 유연성을 제공한다는 것이다.

이 코드에서는 메뉴가 동적으로 추가될 수 있도록 메서드를 사용하여 메뉴를 초기화하므로, 새로운 메뉴를 추가하는 것이 훨씬 용이한 것!

package christmas.domain.menu;

import java.util.Collection;
import java.util.List;
import java.util.stream.Stream;

public class MenuBoardInitializer {

    private MenuBoardInitializer() {
    }

    public static MenuBoard initialize() {
        return new MenuBoard(initializeMenu());
    }

    private static List<Menu> initializeMenu() {
        return Stream.of(
                        initializeAppetizer(),
                        initializeMain(),
                        initializeDessert(),
                        initializeBeverage()
                )
                .flatMap(Collection::stream)
                .toList();
    }

    private static List<Menu> initializeAppetizer() {
        return List.of(
                new Menu(MenuType.APPETIZER, new MenuName("양송이수프"), 6000),
                new Menu(MenuType.APPETIZER, new MenuName("타파스"), 5500),
                new Menu(MenuType.APPETIZER, new MenuName("시저샐러드"), 8000)
        );
    }

    private static List<Menu> initializeMain() {
        return List.of(
                new Menu(MenuType.MAIN, new MenuName("티본스테이크"), 55000),
                new Menu(MenuType.MAIN, new MenuName("바비큐립"), 54000),
                new Menu(MenuType.MAIN, new MenuName("해산물파스타"), 35000),
                new Menu(MenuType.MAIN, new MenuName("크리스마스파스타"), 25000)
        );
    }

    private static List<Menu> initializeDessert() {
        return List.of(
                new Menu(MenuType.DESSERT, new MenuName("초코케이크"), 15000),
                new Menu(MenuType.DESSERT, new MenuName("아이스크림"), 5000)
        );
    }

    private static List<Menu> initializeBeverage() {
        return List.of(
                new Menu(MenuType.BEVERAGE, new MenuName("제로콜라"), 3000),
                new Menu(MenuType.BEVERAGE, new MenuName("레드와인"), 60000),
                new Menu(MenuType.BEVERAGE, new MenuName("샴페인"), 25000)
        );
    }

}

단, 확장 가능성이 없다면?

현재 미션에서는 확장 가능성이 없기 때문에 Enum을 활용할 수 있었을 것 같다.
Enum의 여러 활용을 학습해볼 수 있기때문에!
아마 최종 코딩테스트를 준비하면서 계속 들게될 고민일 것 같은데,

확장 가능성에 맞춰진 설계 vs 요구사항을 잘 준수하고, 그 안에서 객체지향, TDD, 프로그래밍 지식 학습

이 중간을 잘 찾아서 구현해야할 것 같다.

다만, 미션의 요구사항을 떠나, 기존의 코드를 변경하지 않으면서, 기능을 추가할 수 있도록 설계가 되어야한다는 '좋은 설계'의 관점에서 고민해보면 좋을만한 내용인 것 같다!

📌 Badge 결정을 Badge에서 하지 말자?

나는 Badge를 반환하는 메서드를 Badge 내에 정적 팩토리 메서드로 구현했다.
하지만 리뷰를 보고 찾아보니, Enum의 정적 팩토리 메서드는 간단한 인스턴스를 반환하는 데 사용되는 것이 적절해보였다.

그렇다면 이런 다소 복잡한 메소드는 어디로 가야할까?
바로, Badge 등급을 결정하는 객체를 만들어 위임하는 것이다!

그 이유는,

  1. 관리와 유지보수 측면에서 더 유용할 수 있다.
  2. Badge 등급을 결정하는 로직은 특정 객체에 속할 수 있고, 객체의 책임으로서 더 적합할 수 있다.
  3. 객체지향 설계 원칙 중 하나인 '단일 책임 원칙'에도 보다 부합한다.
    • Badge Enum은 여전히 뱃지를 나타내는 역할을 수행하면서, BadgeSelector는 뱃지를 결정하는 로직을 담당한다.

📌 책임... 책임이 나눠진다면 객체를 분할해보기!

이번엔 가격, 뱃지, 증정 상품 등 뭔가를 결정하는 메소드가 많이 필요했고 나는 주문 그룹과 할인 요약, 이 두 객체 안에서 대부분 구현했던 것 같다.
그러다 다른 분들의 코드를 보니 좀 더 분할되어있는 것을 발견하면서 고민이 든 지점이다. 그렇다면 어떤 연습이 이 분할에 도움이 될까?

  1. 객체지향 생활체조 규칙 6. 모든 엔티티를 작게 유지 : 한 클래스를 50줄로 제한
  • 객체가 한 가지 일만 담당하도록 책임과 역할 단위로 객체를 분리해내기 위해
  1. 클래스에 getter 를 제외하고, public 메서드는 최대한 하나만 유지하도록 연습

📌 에러메시지의 광범위성

이 부분은 이펙티브 자바 해당 부분을 읽어보면서 파악을 해봐야할 것 같다...!

📌 inputValidator의 과도한 책임

원래는 구분자가 중복되거나 맨 앞, 뒤에 있지 않은지 등을 검증하는 메소드를 inputValidator 내에 직접 구현했는데 리뷰에서 아이디어를 얻어 개선해보려고한다.

  1. 공백등의 기본적인 문자열 입력을 검증하는 StringValidator
  2. 문자열 중복, 시작 문자열 검증 등의 일반적인 검증을 담당하는 GeneralValidator로 기존의 InputValidator의 메소드들을 분리해보았다!

📌 검증을 도메인에서 분리했을때의 장점?

객체의 비즈니스 로직에서 검증이 분리되었기 때문에, 객체가 수행해야하는 비즈니스 로직에 집중해서 읽을 수 있다는 점 정도인데, 막상 또 리뷰를 받고 생각해보니 이게 충분한 이유가 될까? 하는 생각이 든다.

좀 더 고민이 필요한 부분...!

📌 어느정도 포장해서 보내야할까

이 부분은 평소에 고민이 있던 부분이라 오갔던 리뷰를 스크랩해놓고 정리 해보려고 한다!

2개의 댓글

comment-user-thumbnail
2023년 12월 2일

해당 리뷰에서 겪어보아도 좋을 것 같다고 하신 것 얼마 있지 않아 깨달았다...

커비 글 너무 재밌게 쓰시네욬ㅋㅋ
프리코스 리뷰를 꼼꼼히 정리해주셔서 저도 이 포스팅 보면서 같이 공부하고 있습니다 🤣

1개의 답글