[1팀 이은지] Chapter 2-2. 디자인 패턴과 함수형 프로그래밍#44
Conversation
- App.tsx의 매직넘버들을 의미있는 상수로 교체 - 도메인별 constants 파일 분리 (coupon, cart, discount, product 등) - 코드 가독성 및 유지보수성 향상
- 상품/쿠폰 데이터를 별도 파일로 분리 (src/basic/data/) - useLocalStorage 훅을 [storedValue, setValue] 형태로 단순화 - 함수형 업데이트 로직 개선 및 타입 안전성 강화
- 외부 상태 의존성을 명시적 파라미터로 변경 - 단일 책임 원칙에 따라 로직 분리 (재고확인, 포맷팅, 권한확인) - productId로 price 조회하도록 개선하여 중복 파라미터 제거 - 테스트 가능성 및 재사용성 향상
…CartTotal, getRemainingStock 순수 함수로 분리
JunilHwang
left a comment
There was a problem hiding this comment.
안녕하세요, 훌륭한 리팩토링 작업과 상세한 회고/문서화 잘 보았습니다. 전체 구조 재편, Hook/모델/유틸 분리, 테스트 보강까지 꼼꼼하게 진행하신 점 매우 인상적입니다. 아래는 PR을 실제 GitHub 리뷰 코멘트로 남기실 수 있도록 정리한 종합 피드백(요약)과 상세 피드백(문제 정의 · AS-IS · TO-BE · 코드 예시)입니다.
많은 분량이라 핵심 우선순위 → 설계/구조/패턴 이슈 → 개선안(코드 포함) 순으로 정리했습니다.
주의: 일부 제안은 설계 방향 제시용이며, 코드 샘플은 개념/패턴 설명 목적입니다.
질문에 대한 답변 (PullRequestBody 기반 요약)
-
컴포넌트 분리 수준
- 전반적으로 적절합니다. App.tsx를 라우팅/조합만 담당하도록 축소한 결정(Advanced/App 리팩토링)은 옳습니다.
- 관리자 영역의 Tabs / AdminSection / ProductAdmin 분리는 적정 수준(재사용성과 가독성 균형)을 잘 맞추셨습니다.
- 다만 ProductCard / ProductList 등에서 UI/엔티티 경계(예: formatPrice 의 isAdmin 분기 등)가 일부 남아 있어, UI-엔티티 책임을 더 엄격히 분리하면 테스트·패키징·이식성에서 이득입니다.
-
Icon 관리 불편함
- 현재 네임스페이스 객체(Icon.cart 등)는 직관적이고 간단합니다. 다만 아이콘 추가 시 수동으로 ICONS 맵을 업데이트해야 하는 번거로움이 있습니다.
- 권장: 파일 기준 자동 바렐(index export) 또는 빌드 시 자동 생성(scripts) + 각 아이콘을 개별 컴포넌트로 두고 index에서 재수출. 또는 svg 폴더 스캔하여 런타임/빌드 시 맵을 구성하는 스크립트 사용.
- 예시: 각 아이콘 파일을 src/icons/plus.tsx 등으로 두고 icons/index.ts에서 export * as Icon from './plus' 식의 바렐을 자동생성하면 추가 작업을 1번으로 줄일 수 있습니다.
-
NotificationBoundary 패턴 실용성
- "에러를 던져 전역에서 캐치" 방식(NotificationError)은 흥미롭지만, 제어 흐름이 중단되는 부작용(동작 순서 보장 문제), 테스트/동작 예측성 저하, 그리고 이벤트-에러 처리 복잡성 때문에 실무에서는 잘 사용되지 않습니다.
- 권장 대안: 알림을 발생시키는 전용 서비스/컨텍스트 또는 전역 액션(예: notificationStore/atom, eventEmitter, 또는 useNotification 훅)을 사용해 명시적으로 push. 비동기적인 notifyAsync(Promise.reject)로 우회하는 방법도 있으나, 더 간단한 것은 throw 대신 notify(...) 호출입니다.
- 예: useNotification({ notify })로 통일, throw를 제거하고 비동기 신호가 필요하면 Promise 기반 API 또는 setTimeout 기반 비동기화만 사용.
종합 피드백 (PR 파일 변경 분석 결과 — 키워드)
-
핵심 키워드(요약)
- 모듈화: features-based 구조로 개선 (긍정)
- 단일 책임(SRP): App → 페이지/훅/모델 분리 (긍정)
- 응집도(Cohesion) / 결합도(Coupling): 전반적으로 개선했으나 일부 영역은 낮은 응집도, 높은 결합도 남음 (알림/상품/장바구니의 강한 상호의존)
- 상태 라이브러리 의존성: Jotai 도입으로 일부 훅(앱 전반)이 Jotai에 의존 (추상화 필요)
- Error-as-notification 패턴: 흥미롭지만 제어 흐름 관점에서 위험
- 아이콘 관리: 구조는 좋으나 자동화/확장성 개선 가능
- 테스트 격리: Jotai Provider로 격리 적용(수정됨) — 좋음
-
PullRequestBody의 회고(“과제 셀프회고”, “신경 쓴 부분”)에 대한 피드백(인사이트 중심)
- 잘하셨습니다: 과정 중심 기록, docs로 리팩토링 과정 문서화, 명확한 폴더 구조 개선 타겟(Feature-based) 선택 — 모두 좋은 학습 포인트입니다.
- 인사이트 확장 질문 (더 생각해볼 거리):
- "어디에 무엇이 있는지 모른다" 문제를 해결한 후, 변경이 생겼을 때 연쇄적으로 변경되는 곳은 어디인가요? (변경 영향 범위 매핑)
- 현재 각 feature의 public API(외부에 노출되는 훅/컴포넌트/모델)는 무엇인가요? 패키지화 시 어떤 항목만 공개해야 하나요?
- NotificationError와 같은 전역 전략을 채택할 때, 실패 복구 시나리오(예: 트랜잭션 롤백)를 어떻게 모델링할 것인가요?
-
PullRequestBody의 “리뷰 받고 싶은 내용”에 대한 답변
- 컴포넌트 분리 수준: 적당. 다만 UI와 엔티티(비즈니스)를 더 엄격히 분리하세요. (예: formatPrice → 모델에 옮기고, UI는 표시만)
- Icon 사용 불편함: 자동 바렐 + 파일명 규칙 + 타입으로 관리. (예: icons/generated/index.ts 생성 스크립트)
- NotificationBoundary 실용성: 실무에서는 권장하지 않음. 대안: notify Service / Context / Atom 으로 통합. (아래 상세에 코드/예시 제공)
상세 피드백 (파일별·개념별)
우선, 핵심 개념 정의(간단):
-
응집도(Cohesion) — 여기서 정의한 규칙(요청하신 대로)
- 정의: 하나의 모듈(파일/디렉터리/패키지)이 책임(변경 이유)을 얼마나 단일하게 가지는가.
- 평가 규칙(제안):
- 변경에 대한 파일/코드의 추가·수정·삭제 동선이 짧은가? (변경 시 여러 폴더를 옮겨다녀야 하면 낮은 응집도)
- 라이브러리(패키지)로 떼어낸다고 했을 때, 관련 파일을 매끄럽게 떼어낼 수 있는가? (의존성이 많아 함께 떼기 어렵다면 응집도 낮음)
- 측정: 변경 시 수정해야 할 파일 수, 외부 의존성 수
-
결합도(Coupling)
- 정의: 모듈(함수/컴포넌트)이 다른 모듈과 얼마나 직접적으로 연결되어 있는가. 낮을수록 좋음.
- 좋은 설계: 인터페이스(콜백, 명시적 훅/서비스)를 통해 결합도를 낮춤.
- 나쁜 예시:
- useAddProduct(addNotification) — addNotification 구체 구현(용어/방법)에 결합
- 좋은 예시:
- useAddProduct({ onSuccess, onError })
이제 PR에서 발견된 주요 항목별로 문제(정의)와 개선(AS-IS/TO-BE + 코드)을 제시합니다.
1) 전역 알림(Notification) 패턴 — "에러 던지기" vs "서비스 호출"
문제 정의
- 현재: throwNotificationError 유틸을 만들어 여러 도메인에서 throw new NotificationError(...) 하고, NotificationBoundary가 전역 에러/Promise 이벤트를 수신해 알림으로 처리.
- 문제:
- 제어 흐름이 예기치 않게 중단된다(동기 throw).
- 테스트 환경에서 unhandled rejection/에러로 문제가 생김.
- 호출 지점에서 "언제 로직이 중단되는지"를 명확히 알기 어렵다.
- 이벤트(전역 error/unhandledrejection)에 의존하면 테스팅·해결 경로가 복잡해짐.
AS-IS (간략)
// 사용부
if (somethingBad) {
throwNotificationError.error("메시지"); // throw NotificationError
}
// NotificationBoundary listens to window.onunhandledrejection / onerrorTO-BE(권장 1) — 명시적 notification 서비스 (동기/비동기 모두 안전)
- 장점: 제어 흐름 명확화, 테스트 용이, side-effect가 한 곳에 집중
- 구현 아이디어: useNotification 훅 + notify 함수
예시 코드 (TO-BE)
// src/shared/notification/notificationService.ts
type NotificationType = "success" | "error" | "warning";
export const notificationService = {
notify: (type: NotificationType, message: string) => {
// Jotai atom / context / eventEmitter에 push
window.dispatchEvent(new CustomEvent("app:notify", { detail: { type, message } }));
},
// 테스트용: 스텁/모킹 유도
};
// 사용부
if (somethingBad) {
notificationService.notify("error", "메시지");
}NotificationBoundary는 window 이벤트 대신 notification store(atom/context)을 구독:
// NotificationBoundary
useEffect(() => {
const handler = (e: CustomEvent) => addNotification(e.detail);
window.addEventListener("app:notify", handler as EventListener);
return () => window.removeEventListener("app:notify", handler as EventListener);
}, []);TO-BE(권장 2) — "옵션형" API: notifyAsync (만약 비동기 이벤트 연계가 필요하면)
- notify()는 반환 값으로 Promise를 반환하거나, 내부에서 Promise.reject(new NotificationError(...)) 하여 반드시 NotificationBoundary가 처리하도록 하지 마세요. 단순 notify가 안전합니다.
결론
- Recommendation: throwNotificationError(throw) 패턴은 제거 또는 제한적으로만 사용(트랜잭션 실패 등 정말 예외적 경우). 대부분의 알림은 notificationService.notify(...)로 대체.
2) 상태관리 라이브러리 변경 시(예: jotai → zustand / redux / tanstack-query) 대응 가능성
문제 정의
- 현재 코드에 jotai atoms와 useAtomValue/useSetAtom가 직접 흩어져 있음(ex: src/advanced/features/cart/atoms/cart.atom.ts, useCart 훅에서 useAtomValue).
- 이 상태로 라이브러리 교체하면 많은 파일을 수정해야 함 → 응집도 관점에서 위험.
AS-IS (현재)
// useCart.ts (현재)
const cart = useAtomValue(cartAtom.cart);
const addToCart = useSetAtom(cartAtom.addToCart);문제: 각 훅/컴포넌트가 Jotai API를 직접 사용함 → 전환 비용 큼.
TO-BE(권장) — Adapter/Facade 레이어를 만들어 소비자(컴포넌트)가 라이브러리 세부 구현을 모르도록 함
- 목표: 모든 컴포넌트는 useCart 훅(또는 cartService)에만 의존하고, 내부 구현(Atom/Store)만 바꾸면 됨.
예시 코드
AS-IS → TO-BE 차이 (코드)
// AS-IS: components import directly from atoms
import { useAtomValue } from "jotai";
import cartAtom from "./cart.atom";
const cart = useAtomValue(cartAtom.cart);
// TO-BE: components use facade hook
import { useCart } from "@/features/cart/hooks/useCart";
const { cart, addToCart } = useCart();
// useCart implementation (jotai-backed)
export function useCart() {
const cart = useAtomValue(cartAtom.cart);
const addToCart = useSetAtom(cartAtom.addToCart);
return { cart, addToCart /*...*/ }
}
// If you replace jotai with zustand, only useCart implementation changes.또는 서비스 패턴:
// src/features/cart/cart.service.ts (interface)
export type CartService = {
getCart(): CartItem[];
addToCart(product: Product): void;
removeFromCart(productId: string): void;
subscribe(fn: (cart: CartItem[]) => void): () => void;
};
// jotai 구현
export const createJotaiCartService = (): CartService => { ... };
// zustand 구현
export const createZustandCartService = (): CartService => { ... };
// app 초기화에서 선택 가능한 팩토리로 주입결론
- 지금 단계에서 가장 실용적인 변화: 모든 컴포넌트가 직접 useSetAtom/useAtomValue를 호출하지 않도록 useCart/useProducts 같은 훅들로 추상화하여 중앙에서만 상태 라이브러리를 사용하게 하세요. 그러면 향후 라이브러리 교체가 한 파일(혹은 한 모듈군)에서 끝납니다.
3) 응집도 / 결합도 분석(현재 코드 베이스를 기준으로)
-
긍정적 변화:
- features/별로 디렉토리 구성하여 기능 관련 파일들이 인접한 것은 응집도를 높이는 좋은 선택입니다.
- hooks/models/utils 분리가 명확히 되어 있어 재사용성·테스트성이 향상됨.
-
남아있는 문제(우선순위):
- Notification 관련 코드가 거의 모든 도메인에 침투: 알림 로직이 각 훅/모델에서 직접 호출(throwNotificationError 또는 addNotification). 이건 결합도가 높다는 신호.
- 개선: notificationService/useNotification 추상화.
- 제품 모델(product.model)과 UI가 일부 섞임: formatPrice가 isAdmin 플래그를 참조하거나 UI 포맷을 수행 → 엔티티/뷰 경계 침범.
- 개선: 모델은 순수 데이터(숫자/추천 플래그)만 반환하고 UI 포맷(₩ 기호, 원 단위)는 UI layer에서 처리.
- atoms 간 참조: 예) cart atom이 productsAtom에 접근하는 패턴이 보이거나 products 훅에서 cart를 참조하는 경우 양방향 의존이 생기면 모듈화가 어려워짐.
- 개선: read-only getter API 또는 서비스 패턴으로 한쪽만 참조하게 강제.
- Notification 관련 코드가 거의 모든 도메인에 침투: 알림 로직이 각 훅/모델에서 직접 호출(throwNotificationError 또는 addNotification). 이건 결합도가 높다는 신호.
응집도 측정(사용자 규칙 적용)
- 변경 동선: 일부 흐름(장바구니 변경 → 재고/상품 업데이트 → 알림 처리)은 여러 폴더를 건너감(components → hooks → models → notification). 변경 시 점검해야 할 파일 수가 여전히 많음 → 응집도 보완 필요.
- 패키징 관점: features 단위로 떼어낼 수 있는 정도까지는 왔으나, shared/constants/utils/notification에 대한 의존성이 많아 완전한 패키징(독립 배포)은 아직 부담.
4) addToCart 등 복잡 함수의 리팩토링 (예: 단일 책임, 테스트 용이성)
문제: addToCart가 재고 확인, 알림, setCart 모두 담당 → 높은 결합도, 긴 함수
AS-IS
const addToCart = useCallback((product) => {
const remainingStock = getRemainingStock(product);
if (remainingStock <= 0) {
addNotification('재고가 부족합니다!', 'error');
return;
}
setCart(prev => { /* logic */ });
addNotification('장바구니에 담았습니다', 'success');
}, [cart, addNotification, getRemainingStock]);TO-BE (분리)
- 검증/유효성 함수, 상태 변경 함수, 사이드 이펙트(알림)는 분리
// cart.model.ts (순수 함수)
export function validateAddToCart(product, cart) {
const remaining = getRemainingStock(product, cart);
if (remaining <= 0) return { ok: false, reason: 'OUT_OF_STOCK' };
if (existingQuantity + 1 > product.stock) return { ok: false, reason: 'OVER_STOCK' };
return { ok: true };
}
export function computeNewCartForAdd(product, cart) {
// 순수 계산만 수행
return newCart;
}
// useCart.ts (effectful wrapper)
const addToCart = (product) => {
const validation = validateAddToCart(product, cart);
if (!validation.ok) {
notify({ type: 'error', message: mapReasonToMessage(validation.reason) });
return;
}
const newCart = computeNewCartForAdd(product, cart);
setCart(newCart);
notify({ type: 'success', message: '장바구니에 담았습니다' });
};이점
- model 함수들은 단위 테스트 가능(순수함수)
- useCart는 orchestration만 담당(테스트가 더 쉬움)
- notification은 notify(...)로 추상화되어 결합도가 낮아짐
5) 모듈화·패키지화 관점(Feature → Package)
문제 정의
- 패키지로 만들 때 public API와 내부 구현 분리가 중요(내부에서 shared/constants를 직접 많이 참조하면 못 떼냄).
권장 패턴(예: cart 패키지)
- 디렉토리/파일 구조(패키지화 가능)
- features/cart/
- src/
- index.ts (public API)
- hooks/useCart.ts (public)
- models/cart.model.ts (public 타입/순수함수)
- types/*.ts
- internal/atoms.ts (private)
- utils/*.ts (private)
- src/
- features/cart/
- index.ts에서 노출할 것만 export
- export { useCart } from './hooks/useCart';
- export type { CartItem } from './types/cart.type';
패키지화 체크리스트
- public API는 훅, 타입, 순수 모델 함수만 노출
- 내부는 절대 외부(다른 feature)로부터 직접 import되지 않도록(즉, circular 의존 금지)
- shared 에서 사용하는 것은 범용 유틸만(예: formatPrice util) — 만약 shared가 무거우면 packaging 시 별도 peer dependency로 분리
예시 index.ts
// features/cart/src/index.ts
export { useCart } from './hooks/useCart';
export type { CartItem } from './types/cart.type';
export { calculateCartTotal } from './models/cart.model'; // 순수함수만6) Icon 시스템 개선 (구체 제안 + 코드)
현재: ICONS 객체에 모든 아이콘 수동 등록, Icon 컴포넌트에서 선택
문제: 신규 아이콘 추가 시 ICONS에 수동 추가 필요 → 잦은 번거로움
권장(2가지)
- 정적 바렐 + 타입 자동 추론 (간단)
- 각 아이콘을 src/icons/plus.tsx 등으로 분리
- scripts/gen-icons-index.ts로 icons/index.ts 자동 생성(폴더 스캔)
- 코드:
// icons/index.ts (auto-generated)
export { default as CartIcon } from './CartIcon';
export { default as PlusIcon } from './PlusIcon';
// 그리고 타입 export
export type IconName = 'cart' | 'plus' | ...- 사용:
import { IconName } from '@/icons';
<Icon name={'cart' as IconName} size={6} />- 컴포넌트 직접 전달(더 유연)
- Icon 컴포넌트를 prop으로 받기
// IconWrapper
const Icon = ({ as: Component = DefaultIcon, ...props }) => <svg {...}><Component/></svg>;
// 사용
import CartIcon from '@/icons/CartIcon';
<Icon as={CartIcon} size={6}/>이점
- 아이콘 추가 시 export만 하면 되고, 런타임 레지스트리 관리가 쉬움.
- TypeScript 자동완성 가능.
마무리 요약 (우선순위 작업 권장)
-
Notification 패턴 교체(중요)
- throw 기반 패턴을 notify 호출 방식으로 전환. NotificationBoundary는 더 이상 에러 에벤트에 의존하지 않고 notification atom/context를 서브스크라이브 하도록 변경.
- 테스트 안정성 개선(현재 테스트에서 unhandled rejection 관련 문제를 이미 발견함).
-
상태관리 추상화(중요)
- 모든 훅(useCart/useProducts/useCoupon 등)이 라이브러리 세부 구현을 감추도록 리팩토링. (Adapter/Facade)
- 이렇게 하면 jotai → 다른 라이브러리로 교체 시 비용 최소화.
-
모델/비즈니스(순수함수) 분리(중요)
- validate/compute 등 순수함수로 모델을 정리. UI 레이어는 포맷/렌더링만 담당.
-
아이콘 자동화(중간)
- 아이콘 추가 편의성 개선: 바렐 스크립트 자동생성 또는 as prop 방식 도입.
-
패키지화 준비(중간)
- feature별 public API(훅/타입/순수함수)와 private 내부를 엄격히 분리.
- shared에 의존성 축소(패키지 경계가 명확해야 배포 가능).
구체적인 코드를 포함한 AS-IS / TO-BE 예시 정리(요약)
- Notification: throw → notify
AS-IS:
// any file
throwNotificationError.error('완료!');TO-BE:
// notificationService
notificationService.notify('success', '완료!');- useCart 추상화
AS-IS:
const cart = useAtomValue(cartAtom.cart);
const addToCart = useSetAtom(cartAtom.addToCart);TO-BE:
// components
const { cart, addToCart } = useCart(); // 내부에서 jotai/ zustand 등 사용
// useCart.ts (jotai impl)
export function useCart() {
const cart = useAtomValue(cartAtom.cart);
const addToCart = useSetAtom(cartAtom.addToCart);
return { cart, addToCart }
}- addToCart 분리
AS-IS: addToCart가 validation, state write, notification 수행
TO-BE: validation + compute(모델) / orchestration(useCart) / notification(service)로 분리 (예시 앞에서 제시)
마지막으로 — 당신의 회고/집중한 부분에 대한 추가 피드백(인사이트)
- "과정 중심 글쓰기" 시도와 docs(refactoring-process.md) 작성은 매우 훌륭합니다. 앞으로는 리팩토링 결정의 trade-off(왜 이러면 좋고 저러면 안 되는지)를 문서화할 때, "비용"과 "영향 범위"(예: 변경 시 수정 파일 수)를 정량적으로 적어두면 더 좋습니다.
- NotificationBoundary 같은 실험적 패턴을 도입한 점은 칭찬합니다. 다만 실무 적용 시에는 "예측 가능한 제어 흐름(Predictable control flow)"을 해치는 패턴은 운영 비용을 늘리므로, 그 패턴을 채택하기 전에 실패·회복 시나리오를 문서로 뽑아보세요.
추가 질문(권장)
- Notification을 전역 서비스로 바꿀 경우 테스트 패턴(모킹) 전략은 어떻게 할 건가요?
- Feature를 패키지로 분리할 때 shared/utils / shared/constants 의존성을 어떻게 줄일지 우선순위를 매겨볼까요?
- 현재 Jotai 사용 이유(서로 다른 장단점 고려)를 정리해두셨나요? (tanstack-query/ zustand 와 비교한 선택 근거)
필요하시면
-
- throwNotificationError → notificationService 마이그레이션용 구체적 PR 패치(코드 변경 패치) 작성,
-
- useCart/useProducts 등의 추상화 레이어 예제 구현(간단한 adapter 포함),
-
- Icon 자동 인덱스 생성 스크립트 예시
중 어떤 것을 우선으로 원하시는지 알려주시면 바로 상세 코드 패치 샘플을 만들어 드리겠습니다.
| import { ProductWithUI } from "@/advanced/features/product/types/product"; | ||
| import { PRODUCT } from "@/advanced/shared/constants/product"; | ||
|
|
||
| const cart = atomWithStorage<CartItem[]>("cart", []); |
There was a problem hiding this comment.
저는 atomWithStorage 와 같은 기능을 몰라서 안 썼는데 좋은 것 같습니다
| export const throwNotificationError: Record< | ||
| NotificationType, | ||
| (message: string) => never | ||
| > = { | ||
| [NOTIFICATION.TYPES.ERROR]: (message: string): never => { | ||
| throw new NotificationError(message, NOTIFICATION.TYPES.ERROR); | ||
| }, | ||
|
|
||
| [NOTIFICATION.TYPES.SUCCESS]: (message: string): never => { | ||
| throw new NotificationError(message, NOTIFICATION.TYPES.SUCCESS); | ||
| }, | ||
|
|
||
| [NOTIFICATION.TYPES.WARNING]: (message: string): never => { | ||
| throw new NotificationError(message, NOTIFICATION.TYPES.WARNING); | ||
| }, | ||
| } as const; |
There was a problem hiding this comment.
알림을 모두 Error 로 처리한 이유가 있을까요?
Error 클래스를 상속 받는 부분과, 이름에 Error 를 명시적으로 작성한 부분이 궁금합니다
class NotificationError extends Error
There was a problem hiding this comment.
사실 알림이 Error는 아니지만, 뭔가 전체 도메인에 걸쳐있는 기능을 하위에서 던지면 외부에서 캐치해서 한 곳에서 처리할 수 있는 방법이 없을까? 생각해보다가 에러바운더리처럼 한 번 해보면 어떨까하고 시도해보았습니다!
There was a problem hiding this comment.
지훈님 말씀하신 것처럼 커스텀 이벤트로 처리하면 제가 의도한 바에 더 적절할 것 같습니다.
| const getRemainingStock = (product: Product, cart: CartItem[]): 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.
저는 남은 재고를 '상품의 재고' 라는 관점에서 products 의 역할로 보았는데, cart의 역할로 보신 것이 제 생각과 달라서 어떤 기준으로 분리 하셨는지 궁금해요
There was a problem hiding this comment.
엇, 이 부분은 제 실수인 것 같습니다. stock은 product의 속성이므로, product 도메인 관점에서 관리하는 게 맞다고 생각합니다. 좋은 포인트 짚어주셔서 감사해요!
// 즉시 에러를 던지지 않고 Promise로 감싸서 처리
export const notifyAsync = {
success: (message: string) => {
Promise.reject(new NotificationError(message, 'SUCCESS'));
},
error: (message: string) => {
Promise.reject(new NotificationError(message, 'ERROR'));
},
warning: (message: string) => {
Promise.reject(new NotificationError(message, 'WARNING'));
},
};이 부분도 흥미로운데, 만약 실행중인 코드를 중단시키지 않는 것이 목적이라면 throw 를 사용하지 않고 작성하는 건 어떨까요? |
| )} | ||
| </div> | ||
| <Provider> | ||
| <NotificationBoundary> |
There was a problem hiding this comment.
해당 컴포넌트 역할이 하위 컴포넌트 모두가 알림을 사용하기에 전파하기 위해서 최상단으로 감싸진 구조로 하신걸까요? 제가 생각하는거 이 외에 다른 역할이 있다면 혹은 제가 이해한게 틀렸다면 설명이 듣고싶습니다. 되게 괜찮다고 생각이 드네요
There was a problem hiding this comment.
정석님 해석이 맞습니다! 👍 모든 도메인에 걸쳐있는 알림이라는 녀석을 최상단에서 관리할 수 없을까 고민해보았어요
|
재사용되는 부분에서 컴파운드 패턴을 활용한 부분이 인상적이네요 |
|
enum 과 객체 둘다 사용하셨는데 다르게 사용한 기준이 있는지 궁금합니다 // src/advanced/features/discount/constants/discount.ts
export const DISCOUNT = {
BULK_PURCHASE_BONUS_RATE: 0.05,
MAX_DISCOUNT_RATE: 0.5,
BULK_PURCHASE_THRESHOLD: 10,
} as const;// src/advanced/features/discount/types/discount.type.ts
export enum DiscountType {
AMOUNT = "amount",
PERCENTAGE = "percentage",
} |
| const Icon: React.FC<IconProps> = ({ | ||
| size = 6, | ||
| color = "text-gray-700", | ||
| className = "", | ||
| onClick, | ||
| disabled = false, | ||
| type = "cart", | ||
| }) => { | ||
| const handleClick = () => { | ||
| if (!disabled) { | ||
| onClick?.(); | ||
| } | ||
| }; | ||
|
|
||
| const baseClasses = `w-${size} h-${size} ${color} ${className}`; | ||
| const interactiveClasses = onClick | ||
| ? "cursor-pointer hover:scale-105 transition-transform" | ||
| : ""; | ||
| const disabledClasses = disabled ? "opacity-50 cursor-not-allowed" : ""; | ||
|
|
||
| const IconComponent = ICONS[type]; | ||
|
|
||
| return ( | ||
| <svg | ||
| className={`${baseClasses} ${disabledClasses} ${interactiveClasses} ${color}`} | ||
| fill="none" | ||
| stroke="currentColor" | ||
| viewBox="0 0 24 24" | ||
| onClick={handleClick} | ||
| > | ||
| <IconComponent /> | ||
| </svg> | ||
| ); | ||
| }; |
There was a problem hiding this comment.
아이콘을 공통적인 인터페이스를 가지게 한 부분이 좋은 것 같습니다.
| const ICONS: Record<IconType, React.FC<IconProps>> = { | ||
| cart: CartIcon, | ||
| shop: ShopIcon, | ||
| shopThin: ShopThin, | ||
| minus: MinusIcon, | ||
| image: ImageIcon, | ||
| close: CloseIcon, | ||
| plus: PlusIcon, | ||
| trash: TrashIcon, | ||
| } as const; |
There was a problem hiding this comment.
이 부분에서 아이콘이 늘어나면 번들 사이즈가 불필요하게 커질 수 있을 것 같아요. 아이콘 컴포넌트를 children 인자로 받으면 어떨까요
| <p className="text-sm text-gray-600 mt-1 font-mono">{code}</p> | ||
| <div className="mt-2"> | ||
| <span className="inline-flex items-center px-3 py-1 rounded-full text-sm font-medium bg-white text-indigo-700"> | ||
| {discountType === "amount" |
|
은지님 PR 잘봤습니다 ㅎㅎ 특히 합성컴포넌트라던지 알림 처리 관련된 부분이 저와 비슷한 고민을 하신거같아서 반갑네용. 저는 비즈니스 로직과 알림을 어떻게 분리할지 고민을 했는데 은지님은 알림을 주입받아서 사용하는 부분을 없애려고 하신거군요! 저렇게 바운더리 형태로 처리하신부분이 되게 인상깊어요 👍 |
nemobim
left a comment
There was a problem hiding this comment.
오 이미 5주차부터 FSD를 적용하셨군요..! 은지님 잘 보고갑니다~ 6주차도 화이팅~
| onChange={(e) => { | ||
| const value = e.target.value; | ||
| if (value === "" || regexUtils.isNumeric(value)) { | ||
| setCouponForm({ | ||
| ...couponForm, | ||
| discountValue: | ||
| value === "" | ||
| ? VALIDATION.COUPON_LIMITS.MIN_DISCOUNT_VALUE | ||
| : parseInt(value), | ||
| }); | ||
| } | ||
| }} |
There was a problem hiding this comment.
인라인으로 작성된 이벤트 핸들러들을 별도 함수로 분리하는 것이 로직/뷰 따로 보기 편해서 좋다고 생각합니다..!
또한 자주 쓰이는 함수라면 useCallback으로 메모이제이션하는 것도 성능상 도움이 될 것 같습니닷
There was a problem hiding this comment.
맞아요 극공합니다...저 부분은 미완이에요 흑흑 다음 과제는 더 완성도를 높이겠어욧
| if (value > VALIDATION.COUPON_LIMITS.MAX_DISCOUNT_AMOUNT) { | ||
| setCouponForm({ | ||
| ...couponForm, | ||
| discountValue: | ||
| VALIDATION.COUPON_LIMITS.MAX_DISCOUNT_AMOUNT, | ||
| }); | ||
|
|
||
| throwNotificationError.error( | ||
| `할인율은 ${VALIDATION.COUPON_LIMITS.MAX_DISCOUNT_PERCENTAGE}%를 초과할 수 없습니다` | ||
| ); |
There was a problem hiding this comment.
여기는 할인율이 아니라 할인금액은 ~~~ 원을 초과할 수 없습니다가들어가야될거같은데 맞을까요,,?!
| import { useAtomValue, useSetAtom } from "jotai"; | ||
|
|
||
| import couponAtom from "@/advanced/features/coupon/atoms/coupon.atom"; | ||
|
|
||
| export function useCoupon() { | ||
| const coupons = useAtomValue(couponAtom.coupons); | ||
| const addCoupon = useSetAtom(couponAtom.addCoupon); | ||
| const deleteCoupon = useSetAtom(couponAtom.deleteCoupon); | ||
| const resetCoupon = useSetAtom(couponAtom.resetCoupon); | ||
|
|
||
| const selectedCoupon = useAtomValue(couponAtom.selectedCoupon); | ||
| const setSelectedCoupon = useSetAtom(couponAtom.setSelectedCoupon); | ||
|
|
||
| return { | ||
| coupons, | ||
| addCoupon, | ||
| deleteCoupon, | ||
| selectedCoupon, | ||
| setSelectedCoupon, | ||
| resetCoupon, | ||
| }; | ||
| } |
There was a problem hiding this comment.
atom 폴더를 하나 만들어서 관리하시는건 어떨까요?!
지금은 단순히 atom들만 모여있는데 useCoupon이라는 명명때문에 비즈니스 로직이 있는 hook을 생각하게 됩니다..
// atoms/coupon.atom.ts
export const couponAtoms = {
coupons: coupons,
addCoupon: addCoupon,
deleteCoupon: deleteCoupon,
....
}
이런식으로 atom은 순수하게 상태관리만 한다는 느낌을 주면 더 좋을거같습니다.
There was a problem hiding this comment.
이 부분은 advanced에서 전역 상태를 추가하면서 기존 훅에서 useState로 관리하고 있던 값들을 반환값만 전역상태로 교체해 수정을 최소화한건데, 순수 상태 관리만 담당하는 객체(혹은 모듈)를 별도로 두었으면 더 깔끔했을 거 같아요!
배포 링크
https://angielxx.github.io/front_6th_chapter2-2/
과제의 핵심취지
과제에서 꼭 알아가길 바라는 점
기본과제
Component에서 비즈니스 로직을 분리하기
비즈니스 로직에서 특정 엔티티만 다루는 계산을 분리하기
뷰데이터와 엔티티데이터의 분리에 대한 이해
entities -> features -> UI 계층에 대한 이해
Component에서 사용되는 Data가 아닌 로직들은 hook으로 옮겨졌나요?
주어진 hook의 책임에 맞도록 코드가 분리가 되었나요?
계산함수는 순수함수로 작성이 되었나요?
Component에서 사용되는 Data가 아닌 로직들은 hook으로 옮겨졌나요?
주어진 hook의 책임에 맞도록 코드가 분리가 되었나요?
계산함수는 순수함수로 작성이 되었나요?
특정 Entitiy만 다루는 함수는 분리되어 있나요?
특정 Entitiy만 다루는 Component와 UI를 다루는 Component는 분리되어 있나요?
데이터 흐름에 맞는 계층구조를 이루고 의존성이 맞게 작성이 되었나요?
심화과제
재사용 가능한 Custom UI 컴포넌트를 만들어 보기
재사용 가능한 Custom 라이브러리 Hook을 만들어 보기
재사용 가능한 Custom 유틸 함수를 만들어 보기
그래서 엔티티와는 어떤 다른 계층적 특징을 가지는지 이해하기
UI 컴포넌트 계층과 엔티티 컴포넌트의 계층의 성격이 다르다는 것을 이해하고 적용했는가?
엔티티 Hook과 라이브러리 훅과의 계층의 성격이 다르다는 것을 이해하고 적용했는가?
엔티티 순수함수와 유틸리티 함수의 계층의 성격이 다르다는 것을 이해하고 적용했는가?
과제 셀프회고
💭 과제 시작 전 고민과 목표 설정
4주차 과제에서 AI를 적극적으로 활용하며 과제를 진행했고, AI가 척척 내 요구에 맞게 코드를 짜는 모습을 보면서 희열과 함께 뇌리 저편에서 불안함이 엄습했습니다. 이쯤되니 '본전' 생각이 뇌를 떠나지 않았어요!
이러한 고민 속에서 5주차 과제를 시작하며 일단 우선적으로 근본적인 과제 목표를 정했습니다.
1. 테오가 과제를 통해 느껴보라고 한 것 최대한 느껴보기
1500줄에 달하는 메인 App.tsx를 리팩토링하면서 가장 불편했던 점은, "어디에 무엇이 있는지 모르겠다"는 점이었습니다. 컴포넌트가 추상화되어 있지 않으니 jsx 코드를 봤을 때 앱이 어떤 구성을 하고 있는지 파악이 안되고, 그래서 어디어 무엇이 있는지 알수가 없고, 결국 신규 기능 추가나 수정이 어려울 게 뻔하더라구요.
또 불편했던 점은, 함수와 로직 등 서로 간의 의존도와 결합도가 높아서 분리하려고 해도 분리할 수 없는 상태였다는 점이었습니다. 처음에 단순히 함수부터 분리하고자 함수를 코드에서 잘라내었지만, 외부 상태를 참조하고 있기 때문에 바로 분리할수도 없는 상태였습니다. 하나를 고치면 다른 부분이 깨지는 현상이 계속 발생했습니다.
리팩토링을 진행하면서 엔티티를 다루는 것과 그렇지 않은 것을 구분하는 건 어렵지 않았습니다. 과제의 엔티티가 상품, 장바구니, 쿠폰 등으로 명시적이었기 때문에 엔티티를 다루는지 아닌지 여부를 판단하는데 어렵지 않았습니다.
엔티티를 구분하기 위해 "실제 비즈니스에 존재하는지", "DB에 저장될 정보인지"(여기선 로컬스토리지) 를 생각해보았습니다.
테오가 느끼라고 한 것을 충분히 느끼기 위해 과제를 충실히 따라가고자 했고, 힌트로 주신 폴더를 읽어봤는데 hooks, util은 익숙한데 models 폴더가 익숙치 않아 어떤 역할을 하는 모듈의 집합인지 헷갈렸습니다.
GPT를 통해 학습한 결과 세 폴더의 차이점을 이렇게 요약했습니다.
공용 로직, 비즈니스 로직, 그리고 커스텀 훅 각각 명시적으로 분리하여 관리하고자 했습니다. 이렇게 세가지 계층으로 로직을 분리하고 나니 각 계층의 책임이 명확해지고 어플리케이션 어디에서도 필요한 로직을 재사용할 수 있게 됐습니다.
2. 과제의 결론보다는 과정을 통해 배우기
4주동안 항해하면서 발견한 저의 고질적인 문제가 있습니다. 이력서, 블로그 등 저는 항상 "결과" 위주로 글을 쓴다는 것이었어요. 어렸을 때부터 변수가 많은 스토리 중심의 역사보다는 인풋-아웃풋이 명확한 수학, 과학을 선호했던 성향이 글쓰기에도 영향을 미친 것 같습니다. 하지만 개발자로서 소통과 공유가 중요한 시대에 이는 분명한 약점이라고 느꼈어요.
4주동안 다른 분들의 회고글을 보면 그 과정이 생동감 있게 느껴지고 적절한 비유와 재치로 기술 이야기를 재밌게 풀어내는 분들이 많더라구요. 그런 분들이 참 부러웠고 어떻게 하면 나의 단점을 개선해볼 수 있을까 고민하게 되었습니다. 그래서 이번 과제에는 과정 중심의 글을 써보고자! PR을 작성하고 있는 지금도! 신경쓰며 PR을 작성하고 있고, 이어서 이번 주차 회고 글에서도 실천해볼 계획입니다!
특히, 이번 과제를 진행하면서 각 단계별 작업 내용을 상세하게 문서로 기록하고자 했습니다. (docs/refactoring-process.md) AI가 정리해준 내용이라 미흡한 부분도 있지만, 해당 문서를 통해 각 단계의 작업을 복기할 수 있어서 추후 글을 작성할 때 큰 도움이 되겠더라구요.
이 목표는 PR과 WIL에서 실현해볼 계획이기에 현재 진행 중이고, 목표 달성 여부는 6주차가 시작되고 나서 알 수 있겠네요. (지켜봐주십쇼)
3. 다른 항해러 코드, PR (최대한) 모두 읽어보기
항해를 시작하게 된 큰 동기 중 하나가 다른 주니어 개발자들이 어떻게 하는지 보고 싶다였습니다. 회사에서도 코드 리뷰를 통해 다른 사람들의 코드를 읽어볼 때 인사이트를 많이 얻을 수 있는 것을 느끼고 있습니다. 이러한 목적을 지난 4주동안 잊고 내 과제하는데 급급해서 다른 사람들은 어떻게 하고 있는지 살펴보지 못한 게 참 아쉽습니다. 그래서 이번 주차에는 기필코 다른 분들의 코드를 최대한 모두 읽어보려고 합니다.
이 목표도 과제 제출 후에 실천할 수 있는 내용이라 아직 진행 중이네요! 얼마나 많은 분들의 코드를 읽었는지..다음 주차에 불시에 검문해주셔도 좋습니다. ㅎ_ㅎ
과제를 하면서 내가 제일 신경 쓴 부분은 무엇인가요?
1. 코드 바로 썰고 바로 볶을 수 있는 주방 만들기
개발할 때 동선을 최소화할 수 있는 폴더 구조로 모듈을 관리하고자 했습니다. 처음 과제를 시작할 때는 아래와 같은 기본적인 폴더 구조로 프로젝트를 구성했습니다.
AS-IS: 역할 기반 구조
이 구조는 관심사의 분리를 통해 유지보수성과 확장성을 높이고, 각 기능별로 코드를 체계적으로 관리할 수 있도록 도와줍니다.
이 구조는 모듈 역할 기반으로 폴더가 분리되어 있기 때문에 관심사 분리가 명확하고 각 폴더의 역할이 직관적입니다. 하지만 기능 단위로 개발을 진행할 때 기능 단위로 모듈이 인접해있지 않기 때문에 여러 폴더를 오가며 작업해야하는 단점이 있었습니다.
장바구니 기능을 수정하려면:
마치 요리할 때 재료가 냉장고, 창고, 다락방에 흩어져있는 느낌이랄까...?!
각 유틸, 상수 등을 네임스페이스로 관리하고자 하여 import문은 깔끔하고 가독성이 좋았지만 개발 편의성이나 확장성은 아쉬웠습니다.
준일코치님과의 멘토링에서 이 폴더구조에 대한 피드백을 받고 일명 '개발 동선 최소화'를 위한 새로운 폴더 구조로 개선하고자 했습니다. 처음엔 FSD(Feature-Sliced Design)도 고려했는데, 러닝 커브가 있고 짧은 과제 기간에는 부담스러워서 익숙한 Feature-based 구조를 선택했습니다.
TO-BE: Feature-based 구조
도메인 단위로 폴더 재구성하여 서로 영향 범위가 같은 코드들끼리 인접하도록 해 '바로 야채를 썰고 볶을 수 있게' 개발 동선을 최적화하고자 했습니다.
최상위 구조
주요 기능 도메인
각 기능별 내부 구조
공통 자원
이렇게 폴더 구조를 변경하고 나니 어떤 기능을 수정할 때 이동하는 폴더와 파일 범위가 확실히 좁아졌다는 걸 느꼈습니다. 각 기능 별로 일관된 하위 폴더 구조를 유지하고 있으니 신규 도메인이나 기능이 생길 때 feature 단위로 폴더를 추가하면 돼서 개발편의성도 훨씬 좋아졌습니다.
2. 확장 가능한 공통 컴포넌트 개발
Icon 컴포넌트
Icon 컴포넌트를 일관적인 인터페이스로 쉽게 새로운 아이콘을 추가하고 수정할 수 있도록 설계하고자 했습니다. "가장 단순하면서도 효과적인 방법이 뭘까?" 생각해보다가 모든 SVG 아이콘의 구조가 반복적이고 외부 svg 태그 또한 동일한 형태로 반복되고 있는 걸 발견했습니다.
이를 통해 공통 인터페이스를 가지고 있는 감싸는 메인 Icon 컴포넌트를 만들고, 아이콘 종류마다 각기 다른 path를 가지도록 설계했습니다.
Tabs, AdminSection
합성 패턴을 기반으로 재사용성이 높은 컴포넌트를 구현하고자 했습니다.
3. 개발효율성을 위한 NotificationBoundary 구현
addNotification이 모든 도메인에 걸쳐 사용되어 거의 대부분의 컴포넌트가 props로 받아야 하는 상황이었습니다. 알림을 추가하는 것을 각 사용부에서 처리하는 것이 아니라 외부의 한 곳에서 모아 처리할 방법이 없을까 고민했습니다.
알림을 발생시키는 곳에서는 에러 형태를 던지기만 하고 외부에서 그걸 캐치해서 처리할 수 없을까 고민하게 되었고, 그렇게 떠오른 패턴이 에러 바운더리 구조였습니다.
아키텍처
NotificationBoundary 컴포넌트
2. NotificationError 클래스
3. throwNotificationError 유틸리티
일반 에러와 알림 에러를 구분하여 모두 캐치하여 핸들링 할 수 있고, 알림 관련 로직을 모두 NotificationBoundary에서 처리할 수 있어 관심사가 명확히 분리됐습니다. 알림을 발생시키는 구조도 간편하고 새로운 알림 타입을 추가하기도 좋은 구조지만, 구현하는 과정에서 문제점을 발견했습니다.
어쨌든 알림을 발생시키지만 에러 타입이기 때문에 알림 에러가 발생한 시점에 로직의 흐름이 중단된다는 것이었습니다. 이 문제 때문에 아무데서나 알림을 발생시켜선 안되고 알림을 발생시키전 필요한 로직은 모두 선행시킨 후 마지막에 알림을 발생시켜야 했습니다.
과제를 다시 해보면 더 잘 할 수 있었겠다 아쉬운 점이 있다면 무엇인가요?
함수 리팩토링
아직 개별 함수 단위에서 깔끔하지 못한 코드들이 많은데 시간이 좀 더 있었다면 함수 단위로 리팩토링을 더 진행하고 싶습니다.
비동기 알림 시스템
앞서 언급했던 NotificationError 특성 상 로직을 중단하는 문제를 개선하고 싶습니다. 가능할지는 구현해봐야 알겠지만, Claude가 추천한 방식은 Promise로 감싸서 즉시 에러를 던지지 않게 하는 방법입니다.
동기적 에러에서 비동기 에러로 개선
Promise로 에러를 발생시켜 현재 실행 컨텍스트가 끝난 후 다음 이벤트 루프에서 처리하도록 유도하여 메인 로직이 중단되지 않습니다.
AS-IS: 동기적 에러로 즉시 중단
TO-BE: Promise만 호출하고 계속 진행
NotificationBoundary에서 Promise 감지하는 법
Promise.reject()를 호출하면 이 Promise가 catch되지 않으면 브라우저에서 'unhandledrejection' 이벤트 발생시킵니다.
리뷰 받고 싶은 내용이나 궁금한 것에 대한 질문
컴포넌트 분리 수준에 대한 피드백
제가 선택한 컴포넌트 분리 수준이 적절한지 궁금합니다. 더 세분화해야 하는 부분이나, 반대로 과도하게 분리한 부분이 있을까요?
Icon 사용 불편함
아이콘 추가될 때마다 type과 객체에 추가해줘야 하는 것이 불편합니다. 더 효율적으로 아이콘 컴포넌틀를 관리할 수 있는 설계 구조가 없을지 추천해주세요!
NotificationBoundary 패턴의 실용성
제가 시도했던 에러 기반 알림 시스템이 실무에서도 시도해볼 만한 패턴인지, 혹은 다른 더 나은 전역 알림 처리 방법이 있는지 궁금합니다.