[1팀 한아름] Chapter 2-2. 디자인 패턴과 함수형 프로그래밍#41
Conversation
JunilHwang
left a comment
There was a problem hiding this comment.
안녕하세요, 과제 잘 진행하셨습니다. 구조 분리(components / hooks / models)와 Jotai를 이용한 전역 상태화까지 잘 진행하신 점이 인상 깊습니다. 아래는 PR 전체에 대한 종합 피드백과 파일 단위 상세 피드백(개념 정의 → 문제 → AS-IS → TO-BE 코드)입니다. PR 코드를 실제 Github Review로 붙여 넣기 편하게 Markdown으로 정리했습니다.
주의: 제 코멘트는 코드 전체(많은 파일)를 읽고 판단한 "아키텍처/유지보수성" 중심의 리뷰입니다. 세부 구현 버그는 유닛 테스트와 런타임 실행에서 더 확인하세요.
요약 (한 문장)
- 현재 구조는 엔티티(제품/장바구니/쿠폰)와 UI(컴포넌트)를 분리하려는 의도가 잘 보입니다. 다만 일부 책임(도메인 로직 vs UI 처리)이 혼재된 곳과 전역 상태 라이브러리(현재 Jotai)에 강하게 결합된 부분이 남아 있어, 상태 라이브러리 교체나 패키지화 시 추가 작업이 필요합니다.
목차
- 종합 피드백
- PR 파일 분석 키워드(문제 요약)
- PullRequestBody의 셀프회고/신경 쓴 부분에 대한 인사이트 + 추가 질문
- PR에서 리뷰 받길 원했던 질문(Q1/Q2)에 대한 답변
- 상세 피드백 (개념 정의 → 문제 → AS-IS → TO-BE)
- 결합도/응집도 정의
- 시나리오별 영향 및 구체적 개선 예시 (코드 스니펫)
- 상태관리 라이브러리 변경(jotai -> zustand / redux / tanstack-query)
- 모듈화(패키지화) 시 응집도/결합도 평가와 개선안
- applyCoupon / notification 경계 (특히 Q2 관련)
- 테스트/테스트 래퍼 관련 소소한 코멘트
종합 피드백
- PR에서 관찰한 키워드 (우선순위 낮음→높음 순 아님, 전체 문제 및 개선 포인트)
- 긍정적:
- 엔티티 모델(model)과 훅(hook)으로 분리하려는 시도(모델 폴더와 useLocalStorage 등).
- UI 컴포넌트가 섬세하게 쪼개짐(ProductItem, CartList, NotificationList, Header 등).
- 전역 상태로 Jotai 도입 및 테스트에서 Provider로 격리해 사용함.
- 개선 필요:
- 도메인 로직(특히 쿠폰 적용 유효성, 장바구니 총합 계산)의 위치가 혼재된 곳이 있음 → 일부 컴포넌트가 도메인 결정을 직접 수행.
- Notification(알림)과 도메인(장바구니)이 섞여 있음(알림은 UI 관점인데 도메인 훅/모델이 이를 호출하는 경우 존재).
- 상태 라이브러리 제거/교체 시 영향을 받는 파일/테스트가 많음(전역 훅 추상화가 필요).
- 일부 훅/모델 인터페이스가 UI 관점(addNotification 등)을 직접 기대함 -> 결합도 증가.
- PullRequestBody의 "과제 셀프회고"와 "신경 쓴 부분"에 대한 인사이트
- 잘 하신 점:
- models와 hooks를 분리한 사고(모델은 순수함수, 훅은 상태 갱신 책임)를 체감하고 구현까지 옮기신 점은 좋은 학습 및 설계 경험입니다.
- UI와 상태 로직 분리(특히 작은 컴포넌트 분리, Notification 훅 도입 등)는 유지보수성 측면에서 바람직합니다.
- 더 생각해볼 지점 (질문 형태로 제안):
- selectedCoupon을 useCart에 넣은 결정은 "쿠폰이 cart total에 직접 영향"이라는 근거로 했는데, 이후에 쿠폰이 다른 도메인(예: 주문결제 서비스)에도 적용된다면 어떡할까요? (도메인 경계 재검토)
- models와 hooks의 경계가 “순수함수 vs 상태 갱신”으로 느껴졌다고 했는데, 그 경계 규칙을 문서화하면 다른 팀원이 빠르게 이해할 수 있습니다. 예: model은 pure fn + no side-effect, hook은 state setter + side-effect 허용 등.
- 전역 상태를 어디까지 사용해야 할지 정한 “기준”을 명확히 적어두면 좋습니다. 예: 여러 페이지/컴포넌트에서 사용하는 state는 전역, 해당 컴포넌트 내에서만 사용하면 로컬 등.
- PullRequestBody의 "리뷰 받고 싶은 질문"에 대한 답변 (Q1 / Q2)
Q1: 구조/모델과 훅 경계가 잘 되었는지, 더 나은 기준
- 현재 이해는 적절합니다. 명확한 기준 예시는:
- 모델(models): Entity 배열/객체를 입력받고 새로운 엔티티 상태/결과를 반환하는 순수 함수 세트(테스트 가능, 부작용 없음).
- 훅(hooks): 상태(setState/localStorage 등)와 사이드 이펙트(localStorage sync, 네트워크, 알림 트리거 등)를 수행. 모델을 호출하여 반환값을 state에 반영.
- 컴포넌트: 뷰와 이벤트 바인딩, UI 콜백(onSuccess/onError)을 통해 훅/모델과 상호작용.
- 권장: 모델의 API(입력/반환 형태)를 문서화하고, 훅이 그 모델의 API를 래핑(wrap)하는 패턴을 일관되게 적용하세요.
Q2: applyCoupon 위치(어디에 두는게 좋을지, props 전달 이슈)
- 핵심: 쿠폰 적용 판정(유효성 검사)은 "도메인" 논리입니다. 하지만 알림(addNotification)은 UI/side-effect입니다.
- 권장 패턴:
- 모델: isCouponApplicable(cart, coupon), applyCoupon(cart, coupon) 같은 순수 함수 제공. -> 반환값으로 { newCart, selectedCoupon, error? }를 반환.
- useCart 훅: 모델을 호출하고 state 변경을 수행. 훅의 applyCoupon 함수는 onSuccess/onError 콜백을 인자로 받을 수 있게 설계(또는 Promise를 반환).
- UI 컴포넌트(CouponSelector 등): 단순히 useCart().applyCoupon(coupon, { onSuccess: () => addNotification(...), onError: (msg) => addNotification(msg, 'error') }).
- 이렇게 하면 applyCoupon의 "도메인 판단"은 모델에 있고, 알림은 사용하는 쪽(UI)이 책임집니다. (아래 상세 피드백에서 AS-IS/TO-BE 코드 예시 포함)
상세 피드백 — 핵심 개념 정의 먼저
-
응집도(cohesion) — 여기서 정의한 규칙:
- 변경이 발생할 때(기능 추가/수정/삭제) 수정해야 할 파일/함수의 경로(동선)가 짧을수록 응집도가 높다.
- 모듈(패키지)로 떼어낼 때 의존성이 적고, 외부로 내보내야 할 것(인터페이스)이 명확하면 응집도가 높다.
- 단일 책임(하나의 모듈은 한 가지 역할)에 근접하면 좋다.
- 평가: 현재 advanced 디렉터리 내 엔티티별로 분리(components/hooks/models)하여 응집도가 양호하나, 일부 모듈(예: useCart / CouponSelector / Notification)이 서로의 역할(도메인/UI)을 침범하여 응집도 저하 요인 존재.
-
결합도(coupling):
- 좋은 결합: 모듈 간 인터페이스(간단 명확한 함수 시그니처)를 통해 의존성을 낮춤(ex. useAddProduct({onSuccess, onError}) ).
- 나쁜 결합: 내부 구현 세부사항(예: addNotification 함수 자체)을 직접 요구함.
- 평가: 많은 컴포넌트가 useNotification 훅을 직접 호출해 알림을 담당하도록 한 점은 테스트/라이브러리 교체 시 유연성이 좋지만, 반대로 도메인 훅이 UI 알림을 직접 호출한다면 결합도가 높아짐.
문제 정의 및 시나리오별 영향 분석
시나리오 A — 상태관리 라이브러리 변경(jotai -> zustand / redux / react-query 등)
- 영향을 받는 파일/영역:
- 모든 훅(useCart, useProducts, useCoupons, useNotification 등)과 tests(Provider로 감싸는 부분)
- tests: 현재 advanced/tests/origin.test.tsx는 Jotai Provider로 감쌌음. 라이브러리 변경 시 test wrapper 변경 필요.
- 컴포넌트들은 대체로 custom hooks(useCart 등)의 public API에 의존하므로, hooks 레벨에서만 어댑터를 맞추면 컴포넌트는 변경 없이 유지 가능.
- AS-IS 문제:
- 현재 훅 / store 구현이 Jotai에 밀접하게 작성되어 있으면 바꿀 때 많은 파일이 수정됨.
- TO-BE 권장:
- hooks 레이어를 라이브러리-중립(adapter)으로 만들기: 내부에서 jotai/another store를 사용하되, useCart의 공개 시그니처는 변하지 않도록 유지. 아래 예시참조.
AS-IS (간단 예시)
- imagine useCart.ts 사용 내부에 jotai atom 직접 사용:
// AS-IS (개념)
import { atom, useAtom } from 'jotai';
const cartAtom = atom([]);
export const useCart = () => {
const [cart, setCart] = useAtom(cartAtom);
return { cart, addToCart: (p) => setCart(prev => [...prev, p]) };
};- 이 상태라면 jotai 제거 시 모든 파일 변경 필요.
TO-BE (adapter 패턴)
- hooks만 제공하는 공개 API를 유지하고 내부 구현을 바꿀 때 훅 구현만 교체:
// TO-BE: useCart/index.ts (public API)
export type UseCart = {
cart: CartItem[];
addToCart: (p: Product) => void;
// ... 기타 API
};
// jotai 구현 (implementation/jotai.ts)
import { atom, useAtom } from 'jotai';
const cartAtom = atom<CartItem[]>([]);
export const useCart_jotai = (): UseCart => {
const [cart, setCart] = useAtom(cartAtom);
return {
cart,
addToCart: (p) => setCart(prev => cartModel.addToCart(prev, p))
};
};
// zustand 구현 (implementation/zustand.ts)
import create from 'zustand';
const useStore = create(set => ({
cart: [],
addToCart: (p) => set(state => ({ cart: cartModel.addToCart(state.cart, p) }))
}));
export const useCart_zustand = (): UseCart => {
const cart = useStore(s => s.cart);
const addToCart = useStore(s => s.addToCart);
return { cart, addToCart };
};
// index.ts re-export
export { useCart_jotai as useCart } from './implementation/jotai';
// 나중에 필요하면 re-export를 변경하여 zustand 사용 가능- 장점: 컴포넌트/테스트는 동일한 useCart 시그니처를 사용 → 내부 스토어 구현만 바꾸면 됨.
추가 참고:
- 테스트: renderWithProvider 래퍼를 제공하셨는데(좋음). 상태 라이브러리 변경 시 해당 래퍼만 수정하면 대부분 테스트 수정량이 작아짐.
시나리오 B — 패키지(모듈)로 분리해서 배포해야 하는 경우
- 핵심 질문: "응집도가 높은가? 떼어내기 쉬운가?"
- 현재 구조에서 분리 후보:
- domain (models + hooks): cart-models, coupon-models, product-models + useCart/useProducts hooks
- utils: validators, formatters
- types
- 해야 할 작업/체크리스트:
- public API 정의: 어떤 훅/모델/타입을 외부로 노출할지 명확히 (예: export useCart,useProducts, types).
- 외부 의존성 최소화: Jotai/React에 직접 의존하면 패키지 소비자가 같은 버전의 라이브러리를 사용해야 하므로 peerDependencies로 지정. 더 나은 방법: 스토어 구현을 분리(위 adapter 패턴).
- side-effect 제거: 패키지 내부에 직접 localStorage 또는 DOM 의존이 있으면 배포시 환경 제약(SSR 등)이 생길 수 있음. persistence는 플러그인/옵션으로 주입(inject)하도록 설계.
- 응집도(당신이 정의한 규칙 적용) 평가:
- 변경 동선: 모델 변경(예: addToCart 개선)은 models/cart.ts와 useCart 래퍼만 바꾸면 되는 구조라면 동선 짧음 → 응집도 좋음. 현재 models 존재 및 useCart 사용으로 어느 정도 달성되어 있음.
- 매끄럽게 떼어낼 수 있는가: 현재 컴포넌트들이 useCart/useProducts 같은 추상 훅에 의존하는 구조라면 domain-only 패키지로 떼어내기 수월. 단, useNotification처럼 UI 관련 훅을 domain 훅들이 직접 호출하는 경우(결합도 높음)엔 떼어낼 때 제거/대체가 필요함.
권장 TO-BE 패키지 구조 (예시)
- package root (domain package)
- src/
- models/ (pure functions)
- cart.ts (addToCart, getRemainingStock, applyCouponPure)
- coupon.ts
- product.ts
- hooks/
- adapter/ (jotai/zustand impl)
- index.ts (exported public hook signatures)
- types.ts
- utils.ts
- models/ (pure functions)
- README: public API 문서 및 persistence/notification 주입 방법
- src/
- 패키지 사용자는 UI 쪽에서 addNotification 콜백을 주입하거나 별도 notification 훅을 사용하도록.
구체적 개선 예시 — applyCoupon과 Notification 경계 (Q2 관련, 코드 포함)
문제(AS-IS)
- 현재 CouponSelector 내부 applyCoupon는 cartTotalPrice를 직접 참조하고, setSelectedCoupon 및 addNotification을 직접 호출하고 있음.
- 결과: 쿠폰 유효성(도메인)과 알림(프레젠테이션)이 섞여 있음. 훅/모델 단위로 떼어내기 어려움.
AS-IS (발췌, 요약)
// CouponSelector.tsx (현재)
const applyCoupon = useCallback((coupon) => {
const currentTotal = cartTotalPrice.totalAfterDiscount;
if (currentTotal < ORDER.MIN_FOR_COUPON && coupon.discountType === 'percentage') {
addNotification(MESSAGES.COUPON.MIN_PRICE, 'error');
return;
}
setSelectedCoupon(coupon);
addNotification(MESSAGES.COUPON.APPLIED, 'success');
}, [addNotification, cartTotalPrice]);TO-BE: 도메인/프레젠테이션 분리
- model: 순수함수로 isCouponApplicable, applyCouponPure 제공
// models/cart.ts (pure)
export const cartModel = {
isCouponApplicable: (cartItems: CartItem[], coupon: Coupon) => {
const total = calculateCartTotalFromItems(cartItems); // pure fn
if (coupon.discountType === 'percentage' && total.totalAfterDiscount < ORDER.MIN_FOR_COUPON) {
return { ok: false, reason: 'MIN_PRICE' };
}
return { ok: true };
},
applyCoupon: (cartItems: CartItem[], coupon: Coupon) => {
if (!cartModel.isCouponApplicable(cartItems, coupon).ok) {
return { cartItems, selectedCoupon: null, error: 'NOT_APPLICABLE' };
}
// 실제 할인 계산은 calculateCartTotalFromItems가 적용하도록 selectedCoupon만 반환
return { cartItems, selectedCoupon: coupon, error: null };
}
};- hook: useCart에서 모델을 호출하되, 알림은 콜백으로 분리
// hooks/useCart.ts
export const useCart = () => {
const [cart, setCart] = useLocalStorage('cart', []);
const [selectedCoupon, setSelectedCoupon] = useState<ICoupon|null>(null);
const applyCoupon = (coupon: ICoupon, options?: { onSuccess?: ()=>void; onError?: (msg:string)=>void }) => {
const result = cartModel.applyCoupon(cart, coupon);
if (result.error) {
options?.onError?.(MESSAGES.COUPON.MIN_PRICE);
return false;
}
setSelectedCoupon(coupon);
options?.onSuccess?.();
return true;
};
return { cart, selectedCoupon, applyCoupon, setSelectedCoupon, ... };
};- UI: CouponSelector는 addNotification을 가지고 있고 applyCoupon에 콜백을 전달
// CouponSelector.tsx (TO-BE)
const { applyCoupon } = useCart();
const { addNotification } = useNotification();
const onSelect = (coupon) => {
applyCoupon(coupon, {
onSuccess: () => addNotification(MESSAGES.COUPON.APPLIED, 'success'),
onError: (msg) => addNotification(msg ?? MESSAGES.COUPON.MIN_PRICE, 'error')
});
};장점
- 모델은 순수, 테스트 가능.
- useCart 훅은 state 변경 책임만 가지고, 알림은 UI에 위임(혹은 호출자가 원하는 다른 처리를 할 수 있음).
- 상태 라이브러리 교체 시 모델과 UI는 영향 적음(훅/adapter만 교체).
더 구체적인 결합도 개선 예시 — 알림을 callback으로 바꾸는 경우 (AS-IS/TO-BE 보여주기)
AS-IS (문제)
// useCart.ts (현재 예시)
const addToCart = (product) => {
setCart(prev => cartModel.addToCart(prev, product));
addNotification('장바구니에 담았습니다', 'success'); // UI/notification 호출
};문제: 훅이 addNotification 함수를 내부에서 직접 의존하면, 알림 시스템을 바꾸기 어렵고, 훅의 재사용성이 떨어짐.
TO-BE (권장)
// useCart.ts (TO-BE)
const addToCart = (product, options?: { onSuccess?: ()=>void; onError?: (msg:string)=>void }) => {
setCart(prev => cartModel.addToCart(prev, product));
options?.onSuccess?.();
};
// 컴포넌트에서 호출
const { addToCart } = useCart();
const { addNotification } = useNotification();
addToCart(product, { onSuccess: () => addNotification('장바구니에 담았습니다', 'success') });- 이 패턴은 결합도를 낮추며 훅을 더 범용적으로 만듭니다.
상태 라이브러리 변경 시 실제 마이그레이션 범위(파일 나열)
- hooks/useCart.ts, hooks/useProducts.ts, hooks/useCoupons.ts, hooks/useNotification.ts (스토어/atom 접근부)
- 테스트: advanced/tests/origin.test.tsx (Provider 래퍼)
- components that directly rely on Provider? (대부분 hooks를 통해 데이터 취득함)
- 추천: 하나의 중앙 adapter 폴더(implementation/*)에 스토어별 구현을 모아두고 index에서 re-export 하는 패턴.
예: zustand adapter 예시 (간단)
// implementation/zustand.ts
import create from 'zustand';
const useCartStore = create(set => ({
cart: [],
addToCart: (product) => set(state => ({ cart: cartModel.addToCart(state.cart, product) })),
// ...
}));
export const useCart = () => {
return {
cart: useCartStore(s => s.cart),
addToCart: useCartStore(s => s.addToCart),
};
};- 그리고 public API 모듈에서 이를 re-export.
테스트 관련 짧은 코멘트
- 좋은 점: 테스트에서 Provider를 래핑하는 renderWithProvider를 만들었음(각 테스트에 적용).
- 제안: 공통 테스트 유틸(예: src/setupTests/renderWithProviders.ts)로 분리해 모든 테스트에서 일관되게 사용하면 좋습니다.
- 또한, 테스트에서 로컬스토리지와 시간 기반(토스트 자동 제거) 논리를 좀 더 deterministic하게 하려면 vitest의 fake timers 사용 고려.
마무리 — 종합 권장 액션 리스트 (우선순위 순)
- 모델(cartModel 등)을 더 완전한 pure API로 정리(isCouponApplicable, applyCouponPure, calculateCartTotalsFromItems 등).
- useCart/useCoupons 등 훅은 UI side-effect(알림, 모달 열기)를 직접 호출하지 말고 onSuccess/onError 콜백을 인자로 받도록 변경.
- 상태 라이브러리 추상화 레이어(implementation 폴더)로 분리 — Jotai/Zustand 전환 시 간단하게 교체 가능.
- 패키지화 계획: domain-only 패키지(모델 + hooks의 public API)와 UI 패키지(components)는 분리. persistence & notification은 주입 가능한 인터페이스로 만들 것.
- tests: renderWithProvider 공통 유틸로 분리하고, 상태 라이브러리 변경 시 여기만 수정.
구체적 코드 스니펫 요약 (빠르게 붙여 쓰기용)
- applyCoupon: AS-IS / TO-BE (요약)
AS-IS:
// CouponSelector.tsx
const applyCoupon = (coupon) => {
if (cartTotalPrice.totalAfterDiscount < ORDER.MIN_FOR_COUPON && coupon.discountType === 'percentage') {
addNotification(MESSAGES.COUPON.MIN_PRICE, 'error');
return;
}
setSelectedCoupon(coupon);
addNotification(MESSAGES.COUPON.APPLIED, 'success');
};TO-BE:
// models/cart.ts (pure)
export const isCouponApplicable = (cart, coupon) => {
const total = calculateCartTotalFromItems(cart).totalAfterDiscount;
if (coupon.discountType === 'percentage' && total < ORDER.MIN_FOR_COUPON) {
return { ok: false, reason: 'MIN_PRICE' };
}
return { ok: true };
};
// useCart.ts
const applyCoupon = (coupon, { onSuccess, onError } = {}) => {
const res = cartModel.isCouponApplicable(cart, coupon);
if (!res.ok) { onError?.(res.reason); return false; }
setSelectedCoupon(coupon);
onSuccess?.();
return true;
};
// CouponSelector.tsx (UI)
onChange = (e) => {
const coupon = coupons.find(c => c.code === e.target.value);
if (!coupon) { setSelectedCoupon(null); return; }
applyCoupon(coupon, {
onSuccess: () => addNotification(MESSAGES.COUPON.APPLIED, 'success'),
onError: () => addNotification(MESSAGES.COUPON.MIN_PRICE, 'error')
});
};- state lib adapter 예시
// hooks/useCart/index.ts (public)
export type UseCart = { cart: CartItem[]; addToCart: (p:Product)=>void; ... };
// implementation/jotai.ts
export const useCart_jotai = ():UseCart => { /* jotai impl */ }
// implementation/zustand.ts
export const useCart_zustand = ():UseCart => { /* zustand impl */ }
// index.ts => 현재는 jotai 버전 export, 필요 시 여기만 바꾸면 됨
export { useCart_jotai as useCart };마지막으로, PR의 셀프 회고/질문에 대한 최종 짧은 코멘트:
- models vs hooks 이해는 매우 적절합니다. 다음 단계로는 "모든 모델 함수는 순수함수, 훅은 상태 접합부/side-effect 자리"라는 규칙을 코드베이스 전반에 적용하고 문서로 남겨보세요.
- selectedCoupon은 "쿠폰이 cart total을 변경"한다면 cart 도메인에 두는 게 적절합니다. 단, 알림은 UI가 담당하도록 분리하세요.
- 전역 상태는 '어디까지 전역으로 올릴지'의 가이드(예: 공유되는 데이터가 2개 이상의 페이지나 deep-nesting된 컴포넌트에서 필요하면 전역)를 정해두면 경험적으로 훨씬 수월합니다.
질문이 더 있으시면(예: useCart 구체적 리팩토링 패치, models 리팩토링 PR 예시, 상태관리 adapter 구현 샘플 등) 원하시는 파일들 대상으로 TO-BE 코드 패치 형태로 제공해드리겠습니다.
| const AdminTabs = ({ activeTab, handleActiveTab }: AdminTabsProps) => { | ||
| return ( | ||
| <nav className="-mb-px flex space-x-8"> | ||
| <button | ||
| onClick={() => handleActiveTab("products")} | ||
| className={`py-2 px-1 border-b-2 font-medium text-sm transition-colors ${ | ||
| activeTab === "products" | ||
| ? "border-gray-900 text-gray-900" | ||
| : "border-transparent text-gray-500 hover:text-gray-700 hover:border-gray-300" | ||
| }`} | ||
| > | ||
| 상품 관리 | ||
| </button> | ||
| <button | ||
| onClick={() => handleActiveTab("coupons")} | ||
| className={`py-2 px-1 border-b-2 font-medium text-sm transition-colors ${ | ||
| activeTab === "coupons" | ||
| ? "border-gray-900 text-gray-900" | ||
| : "border-transparent text-gray-500 hover:text-gray-700 hover:border-gray-300" | ||
| }`} | ||
| > | ||
| 쿠폰 관리 | ||
| </button> | ||
| </nav> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
| const AdminTabs = ({ activeTab, handleActiveTab }: AdminTabsProps) => { | |
| return ( | |
| <nav className="-mb-px flex space-x-8"> | |
| <button | |
| onClick={() => handleActiveTab("products")} | |
| className={`py-2 px-1 border-b-2 font-medium text-sm transition-colors ${ | |
| activeTab === "products" | |
| ? "border-gray-900 text-gray-900" | |
| : "border-transparent text-gray-500 hover:text-gray-700 hover:border-gray-300" | |
| }`} | |
| > | |
| 상품 관리 | |
| </button> | |
| <button | |
| onClick={() => handleActiveTab("coupons")} | |
| className={`py-2 px-1 border-b-2 font-medium text-sm transition-colors ${ | |
| activeTab === "coupons" | |
| ? "border-gray-900 text-gray-900" | |
| : "border-transparent text-gray-500 hover:text-gray-700 hover:border-gray-300" | |
| }`} | |
| > | |
| 쿠폰 관리 | |
| </button> | |
| </nav> | |
| ); | |
| }; | |
| export function AdminTab({ | |
| tabs, | |
| activeTab, | |
| setActiveTab, | |
| }: { | |
| tabs: { | |
| label: string; | |
| value: "products" | "coupons"; | |
| }[]; | |
| activeTab: "products" | "coupons"; | |
| setActiveTab: React.Dispatch<React.SetStateAction<"products" | "coupons">>; | |
| }) { | |
| return ( | |
| <div className="border-b border-gray-200 mb-6"> | |
| <nav className="-mb-px flex space-x-8"> | |
| {tabs.map((tab) => ( | |
| <button | |
| key={tab.value} | |
| onClick={() => setActiveTab(tab.value)} | |
| className={`py-2 px-1 border-b-2 font-medium text-sm transition-colors ${ | |
| activeTab === tab.value | |
| ? "border-gray-900 text-gray-900" | |
| : "border-transparent text-gray-500 hover:text-gray-700 hover:border-gray-300" | |
| }`} | |
| > | |
| {tab.label} | |
| </button> | |
| ))} | |
| </nav> | |
| </div> | |
| ); | |
| } |
지금은 탭의 형태와 구조가 고정적인데 외부에서 변수를 변경함으로써 탭이 늘어나고 줄어들 수 있는 구조로 변경하는 것은 어떨까요?
| export const NOTIFICATION_DURATION = 3000; // 알림 삭제 간격 - 3초 | ||
| export const SEARCH_DELAY = 500; // 검색어 반영 딜레이 - 0.5초 |
There was a problem hiding this comment.
NOTIFICATION_DURATION와 SEARCH_DELAY 같은 시간 관련 상수를 constants/time으로 분리하신 점은 깔끔하네요!
다만 NOTIFICATION_DURATION는 useNotification에서만, SEARCH_DELAY는 useSearchTerm에서만 사용되고 있습니다.
현재로서는 상수를 constants로 따로 분리해 export하기보다, 각각 사용하는 곳에 선언해두면 해당 훅의 응집도가 더 높아지지 않을까요? 개인적인 의견입니다! 지금도 좋아요
| export const cartModel = { | ||
| /** | ||
| * 적용 가능한 최대 할인율 계산 | ||
| */ | ||
| getMaxApplicableDiscount: (item: ICartItem, cart: ICartItem[]): number => { | ||
| const { discounts } = item.product; | ||
| const { quantity } = item; | ||
|
|
||
| // 기본 할인율 계산 | ||
| const baseDiscount = discounts.reduce((maxDiscount, discount) => { | ||
| return quantity >= discount.quantity && discount.rate > maxDiscount | ||
| ? discount.rate | ||
| : maxDiscount; | ||
| }, 0); | ||
|
|
||
| // 10개 이상 구매하는 상품이 있는지 확인 (대량 구매) | ||
| const hasBulkPurchase = cart.some( | ||
| (cartItem) => cartItem.quantity >= DISCOUNT.BULK_THRESHOLD | ||
| ); | ||
| if (hasBulkPurchase) { | ||
| return Math.min(baseDiscount + DISCOUNT.BULK_BONUS, DISCOUNT.MAX_RATE); // 대량 구매 시 추가 5% 할인 (최대 할인율 - 50%) | ||
| } | ||
|
|
||
| return baseDiscount; | ||
| }, | ||
|
|
||
| /** | ||
| * 개별 아이템의 할인 적용 후 총액 계산 | ||
| */ | ||
| calculateItemTotal: (item: ICartItem, cart: ICartItem[]): number => { | ||
| const { price } = item.product; | ||
| const { quantity } = item; | ||
| const discount = cartModel.getMaxApplicableDiscount(item, cart); | ||
|
|
||
| return Math.round(price * quantity * (1 - discount)); | ||
| }, | ||
|
|
||
| /** | ||
| * 장바구니 총액 계산 (할인 전/후, 할인액) | ||
| */ | ||
| calculateCartTotal: ( | ||
| cart: ICartItem[], | ||
| selectedCoupon: ICoupon | null | ||
| ): { | ||
| totalBeforeDiscount: number; | ||
| totalAfterDiscount: number; | ||
| } => { | ||
| let totalBeforeDiscount = 0; | ||
| let totalAfterDiscount = 0; | ||
|
|
||
| cart.forEach((item) => { | ||
| const itemPrice = item.product.price * item.quantity; | ||
| totalBeforeDiscount += itemPrice; | ||
| totalAfterDiscount += cartModel.calculateItemTotal(item, cart); | ||
| }); | ||
|
|
||
| // 선택된 쿠폰이 있으면 쿠폰 적용 | ||
| if (selectedCoupon) { | ||
| if (selectedCoupon.discountType === "amount") { | ||
| totalAfterDiscount = Math.max( | ||
| 0, | ||
| totalAfterDiscount - selectedCoupon.discountValue | ||
| ); | ||
| } else { | ||
| totalAfterDiscount = Math.round( | ||
| totalAfterDiscount * (1 - selectedCoupon.discountValue / 100) | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| return { | ||
| totalBeforeDiscount: Math.round(totalBeforeDiscount), | ||
| totalAfterDiscount: Math.round(totalAfterDiscount), | ||
| }; | ||
| }, | ||
|
|
||
| /** | ||
| * 장바구니 상품 수량 변경 | ||
| */ | ||
| updateQuantity: ( | ||
| cart: ICartItem[], | ||
| productId: string, | ||
| quantity: number | ||
| ): ICartItem[] => { | ||
| if (quantity <= 0) { | ||
| // 수량 0 이하면 장바구니에서 제거 | ||
| return cart.filter((item) => item.product.id !== productId); | ||
| } | ||
|
|
||
| return cart.map((item) => | ||
| item.product.id === productId ? { ...item, quantity: quantity } : item | ||
| ); | ||
| }, | ||
|
|
||
| /** | ||
| * 장바구니에 상품 추가 | ||
| */ | ||
| addToCart: (cart: ICartItem[], product: IProductWithUI): ICartItem[] => { | ||
| // 이미 장바구니에 존재하는 상품 처리 | ||
| const existingItem = cart.find((item) => item.product.id === product.id); | ||
|
|
||
| if (existingItem) { | ||
| const newQuantity = existingItem.quantity + 1; | ||
|
|
||
| // 재고 초과 시 기존 cart 반환 | ||
| if (newQuantity > product.stock) { | ||
| return cart; | ||
| } | ||
|
|
||
| // 수량만 업데이트 | ||
| return cart.map((item) => | ||
| item.product.id === product.id | ||
| ? { ...item, quantity: newQuantity } | ||
| : item | ||
| ); | ||
| } | ||
|
|
||
| // 장바구니에 없는 상품이면 새 아이템 추가 | ||
| return [...cart, { product, quantity: 1 }]; | ||
| }, | ||
|
|
||
| /** | ||
| * 장바구니 상품 제거 | ||
| */ | ||
| removeFromCart: (cart: ICartItem[], productId: string): ICartItem[] => { | ||
| return cart.filter((item) => item.product.id !== productId); | ||
| }, | ||
|
|
||
| /** | ||
| * 남은 재고 계산 | ||
| */ | ||
| getRemainingStock: (product: IProductWithUI, cart: ICartItem[]): number => { | ||
| // 상품 재고 수 - 카트에 담긴 상품 수 | ||
| const cartItem = cart.find((item) => item.product.id === product.id); | ||
| const remaining = product.stock - (cartItem?.quantity || 0); | ||
|
|
||
| return remaining; | ||
| }, | ||
| }; |
There was a problem hiding this comment.
헉 model 함수들을 이렇게 객체로 묶는거 되게 좋은 패턴이네요..! 배워갑니다 감사합니다!
| import { useState } from "react"; | ||
|
|
||
| export const useAdminMode = () => { | ||
| const [isAdmin, setIsAdmin] = useState(false); | ||
|
|
||
| const toggleAdmin = () => { | ||
| setIsAdmin((prev) => !prev); | ||
| }; | ||
|
|
||
| return { | ||
| isAdmin, | ||
| toggleAdmin, | ||
| }; | ||
| }; |
There was a problem hiding this comment.
isAdmin 상태를 관리하기 위해 훅을 만드신 것 같은데, 현재 jotai를 사용 중이니 isAdmin을 전역 상태로 관리하는 것은 어떨까요!?
이렇게 하면 useAdminMode 훅은 따로 필요 없을 것 같고, 상태를 다른 컴포넌트들과 쉽게 공유할 수 있어 이전보다 활용도가 높아질 것 같습니다.
또한 전역 상태로 전환하면 컴포넌트 간 props drilling 없이 권한 관련 로직을 일관되게 적용할 수 있어 좀 더 깔끔한 코드가 될 거 같아요
과제의 핵심취지
과제에서 꼭 알아가길 바라는 점
배포 링크
https://areumh.github.io/front_6th_chapter2-2/index.advanced.html
기본과제
Component에서 비즈니스 로직을 분리하기
비즈니스 로직에서 특정 엔티티만 다루는 계산을 분리하기
뷰데이터와 엔티티데이터의 분리에 대한 이해
entities -> features -> UI 계층에 대한 이해
Component에서 사용되는 Data가 아닌 로직들은 hook으로 옮겨졌나요?
주어진 hook의 책임에 맞도록 코드가 분리가 되었나요?
계산함수는 순수함수로 작성이 되었나요?
Component에서 사용되는 Data가 아닌 로직들은 hook으로 옮겨졌나요?
주어진 hook의 책임에 맞도록 코드가 분리가 되었나요?
계산함수는 순수함수로 작성이 되었나요?
특정 Entitiy만 다루는 함수는 분리되어 있나요?
특정 Entitiy만 다루는 Component와 UI를 다루는 Component는 분리되어 있나요?
데이터 흐름에 맞는 계층구조를 이루고 의존성이 맞게 작성이 되었나요?
심화과제
이번 심화과제는 Context나 Jotai를 사용해서 Props drilling을 없애는 것입니다.
어떤 props는 남겨야 하는지, 어떤 props는 제거해야 하는지에 대한 기준을 세워보세요.
Context나 Jotai를 사용하여 상태를 관리하는 방법을 익히고, 이를 통해 컴포넌트 간의 데이터 전달을 효율적으로 처리할 수 있습니다.
Context나 Jotai를 사용해서 전역상태관리를 구축했나요?
전역상태관리를 통해 domain custom hook을 적절하게 리팩토링 했나요?
도메인 컴포넌트에 도메인 props는 남기고 props drilling을 유발하는 불필요한 props는 잘 제거했나요?
전체적으로 분리와 재조립이 더 수월해진 결합도가 낮아진 코드가 되었나요?
과제 셀프회고
과제를 하면서 내가 알게된 점, 좋았던 점은 무엇인가요?
전체적인 회고
이전 과제는 폴더 구조를 어떻게 잡을 지 고민을 많이 했었기 때문에 이번 과제는 힌트로 주어진 리팩토링 폴더 구조를 그대로 따라가기로 했다. 사실 이전에 폴더 구조 관련 질문을 남기고 크게 중요하지 않는다는 답변을 받았는데 아직도 매달리고 있는 걸 보면 아직 멀었다는 생각이 든다.. 요즘은 FSD 디자인에 관심이 가서 깊게 공부해보고 싶다.
제일 먼저는 상품 컴포넌트, 알림 모달 컴포넌트, 쿠폰 컴포넌트, svg 아이콘 등 작은 단위의 컴포넌트 분리를 진행했다. 이게 좋은 선택이었는진 모르겠지만 딱 작은 컴포넌트만 분리하고 그 이상은 진행하지 않았기에.. 이후에 걸림돌이 되지 않았던 것 같다.
컴포넌트 분리 후 models 폴더 내에 비즈니스 로직 (순수 함수) 구현을 했는데, 이때 models과 hooks의 차이가 뭔지 잘 이해되지 않았다. 결국 하는 역할은 똑같은거 아닌가? 하는 생각이 들었고.. 이 의문은 useLocalStorage 훅을 구현하고 적용한 이후에 해결되었다. model은 내가 연산 기호를 정의한 거라면 hook은 그 연산 기호를 사용하여 직접 상태 값을 업데이트 해주는 느낌..? 이 구현 순서를 의도하여 힌트를 주신 건가? 하는 생각도 들었다. 힌트를 괜히 주신게 아니구나..!
장바구니에 상품을 추가하는 함수를 기준으로 설명하자면, models의 순수 함수는 장바구니 배열과 추가할 상품을 둘 다 인자로 받아 이미 장바구니에 존재하는 상품인지를 확인한 후 각 상황에 맞는 값을 반환한다. 그리고 hooks에서는 장바구니와 연동된 setCart 함수를 통해 cartModel의 순수 함수를 사용하여 cart 상태를 업데이트해주었다.
hooks 폴더를 구현하면서 가장 고민했던 부분은 아무래도
addNotification 함수 처리인 것 같다. 상태 관리만 담당하는 함수가 ui 관련 처리까지 담당해도 되는가에 대해 오래 고민했는데, 단일 책임 원칙에 따라 역할을 분리하는 것이 맞다고 판단하여 해당 함수가 필요한 컴포넌트 내에서 hooks 함수와 addNotification 함수를 같이 받아와 처리하도록 구현했다.이런 과정으로 최종적으로 위의 코드 같은 함수가 작성되었는데, 겉으로 보면 기존 origin 코드와 크게 달라진 건 없어 보일 수 있다. (실제로 그렇게 생각했었다..) 하지만 상태 관리 로직과 ui 담당 함수를 분리하여 각각의 책임을 분명히 하여 내부 구조가 개선되었다고 생각한다. 의미있는 리팩토링이었다!
hooks 에서의 또 다른 고민은
selectedCoupon의 선언 위치였다. 이름만 보면 당연히 couponModel, useCoupon에 들어가야 할 것 같았지만, 현재 선택된 쿠폰은 장바구니 상태에 제일 큰 영향을 준다. selectedCoupon을 장바구니 상태를 관리하는 함수 외부에 선언하게 되면, 결국 그 쿠폰 값을 사용하기 위해 장바구니 상태 관리 hook이 쿠폰을 인자로 받게 된다. 이게 어쩔 수 없는 당연한 과정인지, 아니면 피해야 할 상황인 건지 확신이 서지 않았지만, 최종적으로 useCart hook 내부에서 selectedCoupon을 선언하여 사용하는 방식을 택했다.advanced 과제에서의 고민은
전역 상태 훅을 어느 컴포넌트 수준까지 사용하는 것이 적절한가에 대한 것이었다. 전역 상태는 여러 컴포넌트에서 공통으로 사용하는 데이터를 효율적으로 관리할 수 있다는 장점이 있지만, 무분별하게 사용하게 되면 모든 컴포넌트가 전역 상태에 의존하게 되어 코드 간 결합도가 높아지고, 상태의 흐름을 추적하기 어려워질 수 있다..이런 식으로 데이터를 가져오는 로직과 ui를 구성하는 역할을 분리하여, 각 컴포넌트는 자신이 필요한 전역 상태만 의존적으로 사용하게 구현했다. 전역 상태 훅의 올바른 사용법인지는 확실친 않지만.. 현재 구조에선 책임이 명확해져 나름의 기준을 잡을 수 있었다.
가장 아쉬움이 남는 부분은 상품 form 로직 부분이다.. 전역 상태 관련 props는 많이 지워졌지만 form 처리 함수에 사용되는 useState 문이 그대로 남아있다. 커스텀 훅으로의 추상화가 필요하다는 걸 인지하고 있었지만, 과제 마무리에 집중하면서 미루고 미루다 결국 리팩토링하지 못했다. 추후에는 이 부분을 폼 전용 커스텀 훅으로 분리하여 좀 더 읽기 쉽고 관리하기 쉬운 구조로 개선하고 싶다. 충분한 시간이 주어졌지만.. 늘 결과물은 아쉬운 것 같다.
이번 과제에서 내가 제일 신경 쓴 부분은 무엇인가요?
models와 hooks의 연결인 것 같다. models로 순수 비즈니스 로직을 분리해본 적이 없어서 models 함수들을 구현하면서도 이게 올바른 방향인지 계속 의문이 들었다.. 특히 hooks와의 경계가 애매하게 느껴졌는데, useLocalStorage 훅을 직접 구현하면서 이 둘의 차이를 체감할 수 있었다. 리팩토링 힌트 주석이 너무 도움됐다! 리팩토링 폴더를 참고하지 않았다면 과거의 내가 어떻게 이번 과제를 진행했을 지 상상도 되지 않는다..
이번 과제를 통해 앞으로 해보고 싶은게 있다면 알려주세요!
이번 과제를 통해 관심사 분리와 전역 상태 관리를 고민하면서 아키텍처에 대한 관심이 생겼다. 특히 FSD와 같은 구조적 설계를 더 깊게 공부해보고 싶다. 또한 추후에는 복잡한 상태나 폼 로직을 커스텀 훅으로 정리하며 재사용성과 가독성을 높이는 연습도 해보고 싶다!
리뷰 받고 싶은 내용이나 궁금한 것에 대한 질문 편하게 남겨주세요 :)
Q1.
과제 힌트로 제공된 리팩토링 구조를 거의 그대로 참고해서 구현을 진행했는데, 제 코드 구조나 로직 분리 방식이 의도한 방향성과 잘 맞게 구성되었는지, 그리고 models와 hooks의 경계나 역할 분리에 대해 더 나은 개선 방향이 있는지 궁금합니다! 제가 이해한 방식이 적절했는지, 혹시 더 나은 기준이 있는지도 알고 싶습니다.
Q2.
리팩토링 힌트에 따르면 쿠폰을 적용하는 함수인 applyCoupon 함수를 hooks (useCart) 내부에 작성하라고 되어있는데요. 해당 함수 내에서 장바구니 및 쿠폰의 상태에 따라 setSelectedCoupon, addNotification 등의 호출 여부가 달라서 각각
basic/pages/CartPage.tsx와src/advanced/components/coupon/CouponSelector.tsx에 작성했습니다.위와 같이 useCart hook 내부에서 현재 장바구니 상태와 인자로 받은 쿠폰의 타입에 따라 쿠폰 적용이 가능한지에 대한 여부를 boolean 값으로 리턴하는 함수를 만들까 했는데, 이래도 결국 isCouponApplicable, setSelectedCoupon, addNotification 함수를 props로 전달하는 건 똑같지 않나 하는 생각에 만들진 않았습니다. (useCart 훅 내에 쿠폰의 상태를 판단하는 함수가 있는게 어울리지 않다고도 생각했습니다.) 현재 제 코드 상태에서 applyCoupon을 hook 내부에서 작성할 수 있는 방법이 있을까요??? 물론 힌트일 뿐이지만, 더 나은 방식이 있는지 알고 싶습니다!!