[우아한테크코스 6기] 프리코스 4주차 회고

⚠️ 게시글 읽기 전 주의할 점
필드나 파라미터 등은 스포 방지를 위해 실제 코드와 다르게 수정하였습니다!

❗우아한프리코스 4주차 미션 종료

한 달간의 일정이 끝나고 프리코스 마지막 미션이 종료되었다.
회고에 앞서 지난 3주차 때 세웠던 Try와 코드리뷰를 먼저 확인해보자!

💪🏻 지난 Try

• void 메서드 설계를 피하자
• README.md를 통해 프로젝트 소개를 야무지게 해보자
• 메서드를 가독성있게 분류하자!
• 구현이 리팩토링보다 먼저다

💬 코드리뷰 종합

• 서비스 레이어의 역할을 고려하며 설계하기 by @Heon0208
• 정적 팩토리 메서드를 무작정 도입은 말자 by @wns312
• assertThat() 내부는 한번에 하나의 테스트만 적용하자 by @jinkshower
• README.md 내 표는 html <table> 을 활용하자 by geoje

---

👍🏻좋았던 점

🗨️ 효율적인 자료구조를 고민하였다.

public Benefits getBenefits() {
    Map<Custom, Integer> discountEvents = new HashMap<>();
    if (!isEnabled()) {
        return new Benefits(discountEvents);
    }
    if (isSatisfiedGiveaway()) {
        discountEvents.put(GIVEAWAY_EVENT, GIVE_AWAY.calculatePrice());
    }
    return new Benefits(discountEvents);
}

혜택 결과들을 종합하기 위해 처음 고안한 건 Map 형태의 결과를 저장하는 것이었다.

이상함을 느낀 건 다음 이벤트의 결과를 저장할 때였다. 내가 고안한 로직은 결과를 담기만 하지, Map의 장점을 하나도 활용하고 있지 않았다.

그래서 혜택 결과들을 필드로 갖는 새로운 클래스와 List를 활용하여 리팩토링을 적용하였다.

public record EventBenefit(Custom custom, int discountAmount) {
}

public record EventBenefits(List<EventBenefit> eventBenefits) {

    public int getTotalBenefits() {
        //...//
    }

    public int getTotalDiscounts() {
        //...//
    }

    public List<EventBenefit> getEnabledBenefits() {
        //...//
    }
}

적절한 자료구조를 사용함으로써 조금 더 가시적이고 효율적으로 로직을 구성할 수 있었다.

---

🗨️ 추상화 클래스를 이용하여 변경에 열린 개발을 설계하였다.

이번 미션의 핵심 로직 중 하나, 이벤트 정책은 고려해야될 점이 많았다.
이벤트 활성화 여부에 따른 화면 출력 여부도 고려해야했고
각각의 다른 할인 정책을 반영해야했다.

처음엔 하나의 클래스로 모든걸 정의하려 하였다.

public class EventValidator {
	//상수 정의//

    private final OrderedMenus orderedMenus;
    private final VisitDate visitDate;
    private final TotalPrice totalPrice;

    public EventValidator(OrderedMenus orderedMenus, VisitDate visitDate, TotalPrice totalPrice) {
        this.orderedMenus = orderedMenus;
        this.visitDate = visitDate;
        this.totalPrice = totalPrice;
    }

    public GiveAway getGiveAway() {
        //...//
    }

    public int getChristmasDiscount() {
        //...//
    }

    public int getWeekendDiscount() {
        //...//
    }

    public int getWeekdayDiscount() {
        //...//
    }

    public int getSpecialDiscount() {
        //...//
    }

    public Benefits getBenefits() {
        Map<Custom, Integer> discountEvents = new HashMap<>();
        if (!isEnabled()) {
            return new Benefits(discountEvents);
        }
		//HashMap 내 추가 로직
        return new Benefits(discountEvents);
    }
}

모든 기능 구현이 끝난 후 내가 짠 코드를 보며 뭔가 찝찝함을 느끼고 있었다. 그러다 문득 JPA의 BaseEntity가 떠올랐다. 추상화 클래스를 이용하여 모든 할인 정책을 바꿔볼까?라는 생각이 들었고 바로 실행하였다.

public abstract class Event {
    protected final CustomField field1;
    protected final CustomField field2;

    protected Event(CustomField field1, CustomField field2) {
        this.field1 = field1;
        this.field2 = field2;
    }

    public abstract EventBenefit getEventBenefit();

    protected abstract boolean isEventEnabled();

    public boolean isTotalEventEnabled() {
        return field2.isEventEnabled();
    }
}

먼저 Event라는 이름의 추상화 클래스를 생성 후 상속 받을 클래스들이 가져야할 추상화 메서드를 정의하였다.

public class ChristmasEvent extends Event {
    public ChristmasEvent(CustomField field1, CustomField field2) {
        super(field1, field2);
    }

    @Override
    public EventBenefit getEventBenefit() {
        int discountAmount = getDiscountAmount();
        return new EventBenefit(CHRISTMAS_D_DAY_EVENT, discountAmount);
    }

    private int getDiscountAmount() {
		//...//
    }

    @Override
    protected boolean isEventEnabled() {
        return isTotalEventEnabled()
                && field2.isVisitDateInRange(CHRISTMAS_EVENT_START_DAY, CHRISTMAS_EVENT_END_DAY);
    }
}

이렇게 구현하니 각각의 이벤트에 대한 변경에 독립적인 구현을 할 수 있었다.

🗨️ HTML 테이블 태그를 활용하였다.

지난 코드리뷰때 제안 받은 HTMl 테이블 태그를 활용하였다.
마크다운 표에 없는 셀 합치기를 할 수 있어 더 가시적으로 표를 정리할 수 있었다.
아래 표 두개만 비교해도 훨씬 가독성이 좋아진 걸 확인할 수 있다.

---

📖아쉬운 부분

🔥추상 클래스로 분리한 로직을 활용하지 못했다.

모든 이벤트들의 할인 정책을 다룰 클래스를 정의하였다.
처음엔 List<Event>로 일급 컬렉션을 생성할 생각을 하였다.
그러나 증정 메뉴 이벤트는 다른 이벤트와 달리 증정 메뉴를 반환하는 메서드가 추가로 있었다.

이로 인해 업캐스팅으로 클래스를 다루지 못하게 되었다.
그 결과로 아래와 같은 처참한 로직이 발생하였다.

public class EventManager {
    private final Event christmasEvent;
    private final Event weekendEvent;
    private final Event weekdayEvent;
    private final Event specialEvent;
    private final GiveawayEvent giveawayEvent;

    public EventManager(CustomField field1, CustomField field2) {
        this.christmasEvent = new ChristmasEvent(field1, field2;
        this.weekendEvent = new WeekendEvent(field1, field2);
        this.weekdayEvent = new WeekdayEvent(field1, field2);
        this.specialEvent = new SpecialEvent(field1, field2);
        this.giveawayEvent = new GiveawayEvent(field1, field2);
    }

    public Giveaway getGiveaway() {
        return giveawayEvent.getGiveaway();
    }

    public List<EventBenefit> getEventBenefits() {
        List<EventBenefit> benefits = new ArrayList<>();
        if (!christmasEvent.isTotalEventEnabled()) {
            return benefits;
        }
        benefits.add(christmasEvent.getEventBenefit());
        benefits.add(weekendEvent.getEventBenefit());
        benefits.add(weekdayEvent.getEventBenefit());
        benefits.add(specialEvent.getEventBenefit());
        benefits.add(giveawayEvent.getEventBenefit());
        return benefits.stream().toList();
    }
}

제출이 완료되고 나서야 증정 메뉴 반환 로직과 할인 정책 로직을 분리하였으면 한번에 다룰 수 있음을 깨달았다. 😭

🔥 DTO의 비즈니스 로직 소유

❌ Model이 가진 필드를 View에서 처리할 땐 무조건 DTO를 거쳐야 한다.

이는 내가 가졌던 편견 중 하나이다.
결론적으로 말하자면 이는 적어도 이번 프리코스에서는 사실이 아니다.
단순히 출력문으로 사용될 거라면 getter를 이용하여 필드를 불러오면 된다.

그래, dto사용? 좋다 이거다. 그러나 나의 설계 의도상 도메인 로직을 담아선 안되었다.

public record EventBenefits(List<EventBenefit> eventBenefits) {

    public int getTotalDiscounts() {
        return eventBenefits.stream()
                .filter(eventBenefit -> !eventBenefit.isGiveawayEvent())
                .map(EventBenefit::discountAmount)
                .mapToInt(Integer::intValue)
                .sum();
    }

    public List<EventBenefit> getEnabledBenefits() {
        return eventBenefits
                .stream()
                .filter(EventBenefit::isEventEnabled)
                .toList();
    }
}

하지만 나는 이러한 실수를 범했다.. 조금만 생각해보면 도메인 로직으로 봐도 괜찮지 않았을까란 아쉬움이 남는다.

---

🫡Keep And Try

마지막 미션이 끝났다해도 Keep, Try 작성은 해야지.
프리코스를 통해 얻게된 Keep 목록과 이번 Try 목록을 정리해보자

👍🏻 Keep

• 코드는 나만이 보기 위해 작성하는 것이 아니다. 클린코드를 지향하자!
  • 결국 실무에선 모두와 코드를 나눠보기 때문에 가독성을 고려하여 작성해야한다.
• 예외 상황에 맞는 예외 메세지를 적용하자
  • 화면 입출력의 경우 정상 입력전까지 입력값 반복을 고려하자
• 객체 지향적으로 처음부터 설계를 염두하자
• 리팩토링보다 구현이 우선이다. 이후에 리팩토링해도 늦지 않는다.
• README.md는 나의 프로젝트를 소개하는 문서이다. 최대한 정성을 담자
• mock을 이용한 테스트를 피하고 싶다면, void 메서드 설계를 지양하자

💪🏻Try

• Enum도 도메인이 될 수 있다! 패키지 정의시 이를 활용하자!
• 원하는 의도로 구현되지 않는다면 기능 분리를 고려하자!
• 설계 의도와 다르게 흘러간다면 해당 개념을 다시 확인하자!

---

프리코스에 대한 총평은 새로운 글로 작성해보고자 한다!
to be continue...