Skip to content

[9팀 신홍준] Chapter 2-1. 클린코드와 리팩토링#57

Open
jun17183 wants to merge 24 commits intohanghae-plus:mainfrom
jun17183:main
Open

[9팀 신홍준] Chapter 2-1. 클린코드와 리팩토링#57
jun17183 wants to merge 24 commits intohanghae-plus:mainfrom
jun17183:main

Conversation

@jun17183
Copy link

@jun17183 jun17183 commented Jul 31, 2025

과제 링크

https://jun17183.github.io/front_6th_chapter2-1/index.basic.html
https://jun17183.github.io/front_6th_chapter2-1/index.advanced.html

과제 체크포인트

기본과제

  • 코드가 Prettier를 통해 일관된 포맷팅이 적용되어 있는가?
  • 적절한 줄바꿈과 주석을 사용하여 코드의 논리적 단위를 명확히 구분했는가?
  • 변수명과 함수명이 그 역할을 명확히 나타내며, 일관된 네이밍 규칙을 따르는가?
  • 매직 넘버와 문자열을 의미 있는 상수로 추출했는가?
  • 중복 코드를 제거하고 재사용 가능한 형태로 리팩토링했는가?
  • 함수가 단일 책임 원칙을 따르며, 한 가지 작업만 수행하는가?
  • 조건문과 반복문이 간결하고 명확한가? 복잡한 조건을 함수로 추출했는가?
  • 코드의 배치가 의존성과 실행 흐름에 따라 논리적으로 구성되어 있는가?
  • 연관된 코드를 의미 있는 함수나 모듈로 그룹화했는가?
  • ES6+ 문법을 활용하여 코드를 더 간결하고 명확하게 작성했는가?
  • 전역 상태와 부수 효과(side effects)를 최소화했는가?
  • 에러 처리와 예외 상황을 명확히 고려하고 처리했는가?
  • 코드 자체가 자기 문서화되어 있어, 주석 없이도 의도를 파악할 수 있는가?
  • 비즈니스 로직과 UI 로직이 적절히 분리되어 있는가?
  • 코드의 각 부분이 테스트 가능하도록 구조화되어 있는가?
  • 성능 개선을 위해 불필요한 연산이나 렌더링을 제거했는가?
  • 새로운 기능 추가나 변경이 기존 코드에 미치는 영향을 최소화했는가?
  • 코드 리뷰를 통해 다른 개발자들의 피드백을 반영하고 개선했는가?
  • (핵심!) 리팩토링 시 기존 기능을 그대로 유지하면서 점진적으로 개선했는가?

심화과제

  • 변경한 구조와 코드가 기존의 코드보다 가독성이 높고 이해하기 쉬운가?
  • 변경한 구조와 코드가 기존의 코드보다 기능을 수정하거나 확장하기에 용이한가?
  • 변경한 구조와 코드가 기존의 코드보다 테스트를 하기에 더 용이한가?
  • 변경한 구조와 코드가 기존의 모든 기능은 그대로 유지했는가?
  • (핵심!) 변경한 구조와 코드를 새로운 한번에 새로만들지 않고 점진적으로 개선했는가?

과제 셀프회고

매우 열 받는 코드

var PRODUCT_ONE = 'p1'
var p2 = 'p2'
var product_3 = 'p3'
var p4 = "p4"
var PRODUCT_5 = `p5`

너무 열 받는 코드입니다. 특히 가장 열 받는 부분은, 뒤늦게 알게 된 백틱. 헛웃음이 나오더라구요. 코드를 보며 다짐한 첫 번째는 어떻게든 이 코드를 물리치자라는 생각이었습니다. 이 코드를 물리치려면 익숙한 포맷으로 수정해야겠다고 판단하였고, 1주차 과제를 떠올리게 되었습니다.

1주차엔 pages와 components 폴더를 통해 ui를 그리는 부분을 나누었고, 옵저버 패턴을 위해 state 폴더를 두고 각 페이지와 컴포넌트에서 state를 구독하였습니다. 이 구조를 그대로 사용하되 준일 코치님의 솔루션을 참고하여 redux와 비슷한 구조로 리팩토링 하기로 했습니다.

그러나,,,


나도 모르는 내 코드

Pasted image 20250729212427

정신 차리고 보니 순식간에 많은 파일이 생겨났고, 각 파일이 어떤 역할인지 스스로도 잘 모르는 지경에 이르렀습니다. 1주차에서 AI에게 마구잡이로 명령을 내릴 때와 비슷한 상황이었습니다. 아무래도 무자비한 더티 코드에 압도되어 빨리 해결해야 한다는 마음이 컸던 것 같습니다.

(맛이 가버린 AI)
Pasted image 20250730020424


가장 익숙한 걸 해보자

처음 과제를 시작할 때 익숙한 포맷으로 수정해야겠다고 생각했는데 어쩌면 1주차에서 사용했던 구조는 아직 익숙하지 않는구나 싶었습니다. 그래서 더욱 익숙한 포맷으로 빠르게 진행하기로 마음 먹었습니다. 항해에는 잘하시는 분들이 워낙 많고 다들 멋지게 리팩토링을 해내겠지만, 저는 욕심을 버리고 가장 익숙한 코드인 전 회사에서 일할 때 코드와 비슷하게 작성해 보기로 했습니다.

프론트엔드 프레임워크를 사용하지 않는 SI 환경에서는 주로 페이지 단위로 코드를 작성하게 됩니다. (업무 분담을 페이지 단위로 하는 경우가 많기 때문...) 그렇기 때문에 한 파일의 코드량은 많고 가독성이 좋지 않을 수도 있습니다. 하지만 특정 기능을 참고하고 싶다면 그 파일만, 혹은 그 폴더만 확인하면 되는 장점이 있습니다.

두 번째로 jQuery 기반의 환경에선 코드를 크게 데이터 관리, 화면 관리, 이벤트 관리 세 가지로 나누어 작성합니다. 그리고 이벤트가 일어나는 곳에서 데이터 업데이트와 화면 업데이트를 모두 진행합니다. 이 점이 상태를 업데이트하면 이에 따라 화면을 업데이트 하는 리액트와의 가장 큰 차이점이라고 생각합니다.

event -> state / event -> render 보다 event -> state -> render가 훨씬 좋은 구조라고는 생각하지만 이번 과제를 처치하기 위해 전자를 택했습니다.

image

큼직큼직하게 Hedaer, SelectBox, CartList, CartTotal, Manual로 나누었습니다. 아마 기존 방식이라면 장바구니에 상품을 추가할 때 state가 바뀔 거고 이를 바라보는 Header에선 상품 수 수정, CartList에선 상품 추가, CartTotal에선 총 가격 계산을 하게 됩니다. 이걸 ADD TO CART 버튼을 클릭하면 직접 해주어야 하는 겁니다.

image

이전 회사 스타일로 코드를 작성하니 적어도 나만큼은 이 코드에 대해 이해도가 높고 가독성이 좋았습니다. 다만 스스로도 좋은 구조가 아니라고 생각했기에 클린 코드에 대해 더 공부하고 이를 적용하고 싶었습니다. 하지만 이미 시간이 얼마 없는 상태라 심화 과제를 서둘러 진행하게 되었고, 이마저도 AI에게 거의 전부 맡기게 되었습니다.


과제를 다시 해보면 더 잘 할 수 있었겠다 아쉬운 점이 있다면 무엇인가요?

이제 4주차지만 아쉬움이 가장 많이 남았습니다. 첫 번째론 클린 코드에 대한 학습을 결과적으로 많이 얻진 못했던 것 같습니다. 클린 코드란 무엇인지 좀 더 공부하고 알아보고 이를 직접 코드로 치며 적용해야 좋은 학습을 했다고 할 수 있을 텐데, 과제 해결을 위해 익숙함을 택하게 되었습니다.

두 번째도 같은 맥락인데, 심화 과제에서 AI 비중이 너무 컸습니다. 테스트 코드 작성부터 이를 통과하기 위한 코드까지 모두 AI만을 통해 진행했습니다. AI가 코드를 작성하고 이 코드를 분석하는 주객전도 상황이 펼쳐졌습니다. 이후 코드를 직접 읽어보며 추가 리팩토링을 진행하였지만 이 과정에서도 클린 코드에 대한 추가 학습 없이 기존에 가지고 있던 코드 스타일로 리팩토링 하였습니다.

아직 클린 코드 주차가 남았으니 더 공부하여 다음엔 멋드러진 코드를 작성하고 싶습니다.


리뷰 받고 싶은 내용이나 궁금한 것에 대한 질문 편하게 남겨주세요 :)

1. 좋은 커스텀 훅은 무엇인지, 커스텀 훅은 어떤 식으로 작성해야 하는지 궁금합니다.

image

위 코드는 자동 할인을 다루는 훅입니다. 저는 번개 세일, 추천 할인은 같은 자동 할인이라는 카테고리로 묶을 수도 있으며 timer를 편하게 관리한다는 점에서 하나의 훅으로 작성했습니다. 하지만 작성하고 보니 코드 길이도 길고 가독성도 안 좋아 보입니다. 무엇보다 역할이 2개인 점에서 좋은 코드가 아닌 거 같다고도 생각했습니다.

위와 같은 경우 어떤 식으로 코드를 작성하면 좋은지 여쭤보고 싶습니다!

2. 컴포넌트는 어느 정도 단위로 나누어야 할까요?

image

위 장바구니 총합 부분만 보더라도 나눌 수 있는 항목이 굉장히 많아 보입니다. 상품 목록부터 Subtotal, Total, 할인, 포인트 등등 잘게 쪼개려면 모두 쪼갤 수 있을 거 같습니다. 하지만 이렇게 작은 규모인데도 나눌 부분이 많은데 위와 같은 항목을 모두 나누어 버린다면 오히려 프로젝트가 너무 비대해질 거 같습니다.

컴포넌트는 어떤 단위로 나누면 좋을까요? 프로젝트의 규모에 따라서도 달라질까요?


토요일) 코드 리뷰 받고 싶은 내용

1. basic > 기능은 render, data, event로 나누고 컴포넌트는 크게 Header, SelectBox, CartList, CartTotal, Manual로 큼직큼직하게 나눈 편인데 어떻게 생각하시나요??

image

2. 리액트 실무 경험이 없어서 여쭤보고 싶은데 컴포넌트 단위를 어느 정도로 나누는 편이신가요? 잘게 나누려면 정말 한없이 잘게도 나눌 수 있을 거 같아 헷갈리더라구요.

@tomatopickles404
Copy link

tomatopickles404 commented Aug 1, 2025

매우 열 받는 코드
var PRODUCT_ONE = 'p1'
var p2 = 'p2'
var product_3 = 'p3'
var p4 = "p4"
var PRODUCT_5 = p5
너무 열 받는 코드입니다. 특히 가장 열 받는 부분은, 뒤늦게 알게 된 백틱.

읽기전에 공감 하고 시작 😅

@tomatopickles404
Copy link

홍준님 배포 url 다시 한번 확인해보셔용

Copy link

@ldhldh07 ldhldh07 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

basic 영역에서 역할별 구분과 작성방식이 좋았습니다! 수고하셨습니다.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

main 로직이 역할에 충실하고 깔끔해서 인상깊네요!

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

바닐라 JS는 바닐라 JS만의 Best Practice가 있을 것 같아요. 라이브러리가 제공하는 API(실행 순서, 상태 변경에 따른 자동 렌더링 등)들이 없는 바닐라 JS 환경에서는 관리하기가 힘들 것 입니다. 이러한 관점에서 홍준님이 작성한 코드는 바닐라 JS에서 작성할 수 있는 Best Practice 같아요!

};

// 장바구니 아이템 액션 처리 (수량 변경, 삭제)
export const handleCartItemActions = (event) => {
Copy link

@ldhldh07 ldhldh07 Aug 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

해당 함수는 조건 분기가 많고 하나의 함수에 여러개의 추상화 단계를 담고 있어서 분리하면 좀 더 읽기 좋을 것 같습니다!

  • 수령 변경과 삭제를 분리해서 따로 조건 분기
  • if ~ else ~ 보다는 early return을 이용한 분기를 활용

if문 중첩 없이 가독성을 늘리고 개선될 수도 있지 않을까 합니다.

export const UI_CONSTANTS = {
COLORS: {
// 할인 관련 색상
SALE_PRICE: 'text-red-500', // 번개세일 할인 가격 색상
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UI도 상수로 나눈 부분이 일관적인 스타일링으로의 확장성도 갖출 수 있겠네요.

@ckdwns9121
Copy link
Member

홍준님 고생하셧어영~~ 맛이가버린 AI 나도 저랬는데 ㅋㅋㅋㅋ..

Copy link

@xxziiko xxziiko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

홍준님~ 요번주 고생 많이 하셨을텐데 매주 성장하는 모습이 보기 좋슴니다,,
솔직한 회고도 인상 깊네용 ㅎㅎ 담주도 파이링 ~

Comment on lines +155 to +164
*/
const App: React.FC = () => {
return (
<AppProvider>
<ShoppingCart />
</AppProvider>
);
};

export default App; No newline at end of file
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 파일에서 가장 메인이 되는 컴포넌트는 이건데, 위에 분리되어 있는 컴포넌트가 너무 커서 이 파일이 어떤 파일을 의미하는건지 잘 드러나지 않는 것 같아요. 위에 있는 컴포넌트를 분리하던가, App 컴포넌트를 상단으로 올리고 export 를 같이 선언하는게 조금 더 드러나지 않을까 생각합니다~

이렇게용

export function App () {
  return (
    <AppProvider>
      <ShoppingCart />
    </AppProvider>
  );
};

그리고 저는 개인적으로 컴포넌트는 변수 할당이 아닌 함수형으로 표현하는게 조금 더 의미에 맞다고 생각해요!

Comment on lines +23 to +46
const renderCartItem = (item: CartItem) => {
const priceDisplay = createCartItemPriceDisplay(item);

return (
<div
key={item.id}
id={item.id}
className="grid grid-cols-[80px_1fr_auto] gap-5 py-5 border-b border-gray-100 first:pt-0 last:border-b-0 last:pb-0"
>
<CartItemImage />

<div>
<CartItemInfo item={item} priceDisplay={priceDisplay} />
<CartItemControls item={item} onQuantityChange={onQuantityChange} />
</div>

<CartItemPrice
item={item}
priceDisplay={priceDisplay}
onRemove={onRemove}
/>
</div>
);
};
Copy link

@xxziiko xxziiko Aug 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이게 하나의 컴포넌트가 되는 거 같은데 분리해서 컴포넌트로 표현하는게 좋을 거 같아요!

startAutoEvents,
} = useAppContext();

const [isManualOpen, setIsManualOpen] = useState(false);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

모달 상태를 훅으로 따로 캡슐화해서 핸들러랑 함께 관리하면 유지보수에 좋을 것 같습니다!

export function useManualOpen () {
   const [isManualOpen, setIsManualOpen] = useState(false);

   const toggleManual = () => {
     setIsManualOpen((prev) => !prev)
    }

   return {
    isManualOpen,
    toggleManual 
   }
}

// 사용
const {isManualOpen, toggleManual} = useManualOpen()

이런식으로요!

Comment on lines +32 to +70
const summaryItems = [];

// 장바구니 아이템들
items.forEach(item => {
summaryItems.push(
<CartItemSummary key={item.id} item={item} />
);
});

// Subtotal 및 할인 정보 추가
if (subtotal > 0) {
summaryItems.push(<SubtotalSection key="subtotal" items={items} />);

// 할인 정보
const discountSummary = (
<DiscountSummary
key="discount"
items={items}
discountInfo={discountInfo}
/>
);
if (discountSummary) {
summaryItems.push(discountSummary);
}

// 화요일 할인
const tuesdayDiscountItem = createTuesdayDiscountItem(tuesdaySpecial, totalAmount);
if (tuesdayDiscountItem) {
summaryItems.push(
<div key={tuesdayDiscountItem.key} className={tuesdayDiscountItem.className}>
<span className="text-xs">{tuesdayDiscountItem.label}</span>
<span className="text-xs">{tuesdayDiscountItem.value}</span>
</div>
);
}

// 배송
summaryItems.push(<ShippingSummary key="shipping" />);
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오호.. jsx를 배열에 담아서 ui 로 표현하는 방식은 신박하네용..? 저는 낯설어서 그런지 나쁜 냄새가 좀 나는 거 같은데..!
리액트는 선언 기반의 UI를 선호하는데, 지금 이 구현 방식은 if문에 따라서 push를 반복하는 명령형에 가까운거 같아요

Comment on lines +29 to +36
<div id="summary-details" className="space-y-3">
<CartSummary
items={items}
discountInfo={discountInfo}
totalAmount={totalAmount}
tuesdaySpecial={tuesdaySpecial}
/>
</div>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

div까지 묶어서 CartSummary 컴포넌트로 분리하지 않고 따로 이렇게 빼신 이유가 있으실까요?

Comment on lines +3 to +20
export const CART_CONSTANTS = {
// 할인 기준 수량
INDIVIDUAL_DISCOUNT_THRESHOLD: 10, // 개별 상품 할인 기준 (10개 이상)
BULK_PURCHASE_THRESHOLD: 30, // 대량구매 기준 (30개 이상)

// 할인율
BULK_PURCHASE_DISCOUNT_RATE: 0.25, // 대량구매 할인율 (25%)
TUESDAY_DISCOUNT_RATE: 0.10, // 화요일 할인율 (10%)

// 개별 상품별 할인율 (10개 이상 구매 시)
INDIVIDUAL_DISCOUNT_RATES: {
'p1': 0.10, // 키보드 10%
'p2': 0.15, // 마우스 15%
'p3': 0.20, // 모니터암 20%
'p4': 0.05, // 노트북파우치 5%
'p5': 0.25, // 스피커 25%
} as const,

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

객체 내부의 키 표현을 통일하는게 좋을 것 같아요! 소문자로 표현할지, 모두 다 대문자로 표현할지?

Comment on lines +24 to +113
const [activeSales, setActiveSales] = useState<SaleEvent[]>([]);
const lightningTimerRef = useRef<number | null>(EVENT_CONSTANTS.TIMER_INITIAL);
const suggestedTimerRef = useRef<number | null>(EVENT_CONSTANTS.TIMER_INITIAL);
const isRunningRef = useRef<boolean>(EVENT_CONSTANTS.EVENT_STATUS.STOPPED);

// 번개세일 이벤트 생성 및 적용
const createLightningSaleEvent = useCallback((): void => {
try {
const randomProduct = getRandomProduct();
const lightningEvent = createSaleEvent(
'lightning',
randomProduct,
DISCOUNT_POLICY.LIGHTNING_SALE_RATE
);

setActiveSales(prev => {
const filtered = filterExistingEvents(prev, lightningEvent);
return [...filtered, lightningEvent];
});
} catch (error) {
console.error('번개세일 이벤트 생성 실패:', error);
}
}, []);

// 추천할인 이벤트 생성 및 적용
const createSuggestedSaleEvent = useCallback((excludeProductId: string): void => {
try {
const suggestedProduct = getRandomProduct(excludeProductId);
const suggestedEvent = createSaleEvent(
'suggested',
suggestedProduct,
DISCOUNT_POLICY.SUGGESTED_SALE_RATE
);

setActiveSales(prev => {
const filtered = filterExistingEvents(prev, suggestedEvent);
return [...filtered, suggestedEvent];
});
} catch (error) {
console.error('추천할인 이벤트 생성 실패:', error);
}
}, []);

// 번개세일 타이머 설정
const setupLightningSaleTimer = useCallback((): void => {
lightningTimerRef.current = setTimeout(() => {
const interval = setInterval(createLightningSaleEvent, AUTO_EVENT_CONFIG.LIGHTNING_SALE_INTERVAL);
lightningTimerRef.current = interval;
}, AUTO_EVENT_CONFIG.LIGHTNING_SALE_DELAY);
}, [createLightningSaleEvent]);

// 추천할인 타이머 설정
const setupSuggestedSaleTimer = useCallback((lastSelectedProductId: string): void => {
if (!lastSelectedProductId) return;

suggestedTimerRef.current = setTimeout(() => {
const interval = setInterval(
() => createSuggestedSaleEvent(lastSelectedProductId),
AUTO_EVENT_CONFIG.SUGGESTED_SALE_INTERVAL
);
suggestedTimerRef.current = interval;
}, AUTO_EVENT_CONFIG.SUGGESTED_SALE_DELAY);
}, [createSuggestedSaleEvent]);

// 자동 이벤트 시작
const startAutoEvents = useCallback((lastSelectedProductId: string): void => {
if (isRunningRef.current) {
console.warn('자동 이벤트가 이미 실행 중입니다.');
return;
}

isRunningRef.current = EVENT_CONSTANTS.EVENT_STATUS.RUNNING;

try {
setupLightningSaleTimer();
setupSuggestedSaleTimer(lastSelectedProductId);
} catch (error) {
console.error('자동 이벤트 시작 실패:', error);
isRunningRef.current = EVENT_CONSTANTS.EVENT_STATUS.STOPPED;
}
}, [setupLightningSaleTimer, setupSuggestedSaleTimer]);

// 자동 이벤트 중지
const stopAutoEvents = useCallback((): void => {
isRunningRef.current = EVENT_CONSTANTS.EVENT_STATUS.STOPPED;

clearTimer(lightningTimerRef);
clearTimer(suggestedTimerRef);
setActiveSales([]);
}, []);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

세션때 테오가 설명해주셨던 예시랑 비슷한 것 같은데, 상태랑 관련 함수랑 응집시켜놓는 방식이 조금 더 읽기 쉬울 것 같아요! 그걸 훅으로 분리하면 더 좋고요!

이런느낌으로다가~

const useSaleEvents = () => {
  const [activeSales, setActiveSales] = useState<SaleEvent[]>([]);
  
  const createLightningSaleEvent = useCallback((): void => { ... }, []);
  const createSuggestedSaleEvent = useCallback((excludeProductId: string): void => { ... }, []);
  
  return { activeSales, createLightningSaleEvent, createSuggestedSaleEvent };
};

const useEventTimers = () => {
  const lightningTimerRef = useRef<number | null>(EVENT_CONSTANTS.TIMER_INITIAL);
  const suggestedTimerRef = useRef<number | null>(EVENT_CONSTANTS.TIMER_INITIAL);
  
  const setupLightningSaleTimer = useCallback((): void => { ... }, []);
  const setupSuggestedSaleTimer = useCallback((): void => { ... }, []);
  
  return { setupLightningSaleTimer, setupSuggestedSaleTimer };
};

Comment on lines +14 to +15
const [cartItems, setCartItems] = useState<CartItem[]>([]);
const { calculatePoints } = usePoints();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이것도 추상화 레벨이 안맞는것 같습니당!
usePoints 훅을 만든 것 처럼 cartItems을 관리하는 훅도 분리해보면 좋을 것 같아요~

Comment on lines +10 to +18
export interface DiscountPolicy {
BULK_PURCHASE_THRESHOLD: number;
BULK_PURCHASE_RATE: number;
INDIVIDUAL_DISCOUNT_THRESHOLD: number;
TUESDAY_DISCOUNT_RATE: number;
LIGHTNING_SALE_RATE: number;
SUGGESTED_SALE_RATE: number;
SUPER_SALE_RATE: number;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이런 타입은 enum을 사용해봐도 괜찮을 것 같네요! 타입 단언을 해도 괜찮을 것 같구요

  • enum
export enum DiscountPolicy {
  BULK_PURCHASE_THRESHOLD = 30,
  BULK_PURCHASE_RATE = 0.25,
  INDIVIDUAL_DISCOUNT_THRESHOLD = 10,
  TUESDAY_DISCOUNT_RATE = 0.1,
  LIGHTNING_SALE_RATE = 0.2,
  SUGGESTED_SALE_RATE = 0.05,
  SUPER_SALE_RATE = 0.25,
}
  • as const
export const DISCOUNT_POLICY = {
  BULK_PURCHASE_THRESHOLD: 30,
  BULK_PURCHASE_RATE: 0.25,
  INDIVIDUAL_DISCOUNT_THRESHOLD: 10,
  TUESDAY_DISCOUNT_RATE: 0.1,
  LIGHTNING_SALE_RATE: 0.2,
  SUGGESTED_SALE_RATE: 0.05,
  SUPER_SALE_RATE: 0.25,
} as const;

// 타입 추론
export type DiscountPolicy = typeof DISCOUNT_POLICY;

@@ -0,0 +1,32 @@
// 기본 엔터티 타입
export type { Product, CartItem } from './entity';
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저는 export 할 때 그냥 * 으로 표현하는데, 조금 더 간결한 방법도 있다! 정도로 공유드려용

export * from './entity';

Copy link

@susmisc14 susmisc14 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

홍준님 4주차도 고생 많으셨습니다. 요번 basic에서 SI 짬바 많이 느낄 수 있었습니다. 바닐라 JS는 바닐라 JS만의 Best Practice가 있고 React는 React만의 Best Practice가 있다고 생각해요. 그런면에서 홍준님은 basic을 Vanilla Practice에 가깝게 하신 것 같아요! 요번주도 고생 많으셨고 다음 과제도 화이팅입니다!!

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

바닐라 JS는 바닐라 JS만의 Best Practice가 있을 것 같아요. 라이브러리가 제공하는 API(실행 순서, 상태 변경에 따른 자동 렌더링 등)들이 없는 바닐라 JS 환경에서는 관리하기가 힘들 것 입니다. 이러한 관점에서 홍준님이 작성한 코드는 바닐라 JS에서 작성할 수 있는 Best Practice 같아요!

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

현재 index.js 파일들이 다른 모듈의 함수들을 export하는 역할과 더불어, initializeData(), setupAllEventListeners()처럼 여러 모듈을 조율하는 실행 로직도 함께 가지고 있는데요.

보통 index.js(배럴 파일)는 해당 폴더의 '공개 인터페이스'를 정의하는 역할, 즉 "우리 폴더는 이런 기능들을 외부에 제공합니다"라고 목록을 알려주는 역할에 집중하는 경우가 많습니다.

index.js 파일은 로직 없이 export에만 집중하도록 역할을 분리하면 어떨까요?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여러 곳에 updateAllUI()가 중복돼서 사용되고 있어요. 함수의 내용이 약간 다르긴 하지만, '무언가 변경되었으니 UI 전체를 새로고침한다'는 목적은 같습니다. 이는 DRY 원칙에 어긋나며, 나중에 새로운 UI 업데이트 로직을 추가할 때 여러 파일을 수정해야 하는 번거로움을 유발할 수 있습니다.

이 함수들을 'UI를 그리는 책임'을 가진 render 폴더의 한 파일로 통합하는 것은 어떨까요? 그렇게 되면 어떤 로직이든 UI 업데이트가 필요할 때 이 통합된 함수 하나만 호출하면 되므로 코드가 훨씬 간결해질 것 같아요.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants