[8팀 정유열] Chapter 2-1. 클린코드와 리팩토링 by Yuyeol · Pull Request #11 · hanghae-plus/front_6th_chapter2-1

Yuyeol · 2025-07-27T09:09:10Z

과제 체크포인트

배포링크

https://yuyeol.github.io/front_6th_chapter2-1/

기본과제

심화과제

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

과제 셀프회고

이슈

[React 마이그레이션] 실시간 가격 업데이트 콜백 덮어쓰기 문제 해결

과제를 하면서 내가 제일 신경 쓴 부분은 무엇인가요?

작업순서를 정하고 그에 따라 작업하기

코드 첫인상

솔직히 이번 과제로 코드 리팩토링을 한다고 했을때 처음 들었던 생각은 "재밌겠다. 얼른 해보고싶다."였습니다. 평소에도 코드를 수시로 정리하고, 가독성을 고려한 코딩 습관을 가지고 있었기에 나름대로 자신도 있었구요.
하지만 약 800줄이 되는 이 코드뭉치를 보고 "이건 도대체 어떻게 동작하는 코드인거지?"라는 생각이 들었습니다.
감상을 간단하게 말해보자면 아래와 같습니다.

의미를 알기도 힘들고, 위치를 옮기기만 해도 에러가 마구 터지는 변수들
곳곳에서 너 고생좀 하라고 일부러 그랬어. 라고 외치는 듯한 하드코딩들
단순히 혼재 라고 표현하기 미안할 정도로 복잡하게 꼬여있는 비즈니스 로직과 UI로직
내가 흔히 만들어내던 함수와 같은 친구라고 부르기 어려울 정도로 규모가 거대하고 복잡한 함수들

진정하고 천천히 작업 순서를 정해보았고 다음과 같은 순서로 작업을 진행하였습니다.
(작업을 진행한 이유도 함께 기록했습니다.)

(1) var → let/const 치환

🚨 코드 스멜

전역 변수 선언: 어디서든 접근 가능해서 의도치 않은 변경 위험
- 🚨 문제 시나리오: 다른 스크립트에서 전역 변수를 선언하거나, 함수 내부에서 실수로 전역 변수를 수정해서 전혀 다른 곳에서 예상치 못한 동작이 발생할 수 있음.
```
var prodList;
var bonusPts = 0;
...
```
var의 재선언이 가능한 특징: 같은 변수를 여러 번 선언하는 패턴이 가능하여 최초 선언인지 아닌지 구분이 어려움
- 🚨 문제 시나리오: 팀원간 다른 의도로 동일 변수명 선언한 경우 의도치 않은 버그를 일으킬 수 있음.
```
function main() {
  var root, header, gridContainer;
  // ... 50줄 후 ...
  var root = document.getElementById('app');
}
```
호이스팅: 전역 변수가 이미 window에 등록되어 있어서 선언 전 출력 시에도 에러가 아닌 undefined 반환
- 🚨 문제 시나리오: 변수를 선언하기 전에 사용해도 에러가 발생하지 않아서 초기화되지 않은 변수로 계산이
  진행되어 NaN이나 예상치 못한 결과가 나오는 버그가 발생할 수 있음.

🔧 개선 후 효과

변수 생명주기 명확화: const는 재할당 불가, let은 재할당 가능을 명시적으로 표현
블록 스코프로 통일: var, let, const 혼용으로 인한 스코프 혼동 방지
호이스팅 우려 배제: 선언 전 접근 시 명확한 에러로 디버깅 용이, 코드 실행 순서가 선언 순서와 일치
리팩토링 안전성 확보: 이후 함수 분리나 모듈화 작업의 기반 마련

(2) 매직넘버 상수화

🚨 코드 스멜

하드코딩된 매직넘버 산재: 할인율, 수량 기준, 타이머 값들이 의미를 알기 어려운 숫자로
흩어져 있음
- 🚨 문제 시나리오: 대량구매 기준을 30개에서 25개로 변경하려고 했는데, 코드 곳곳에 있는 30을 찾아서
  일일이 수정하다가 다른 의미의 30까지 실수로 변경해버리는 상황
```
if (q >= 10) {
  // 10이 뭘 의미하는지 모호
  if (curItem.id === 'p1') {
    disc = 10 / 100; // 키보드 10% 할인인데 왜 굳이 10/100?
  }
}
// 30개가 대량구매 기준이었던가?
if (itemCnt >= 30) {
  totalAmt = (subTot * 75) / 100; // 이건 알아보기 자체가 어렵다
}
```
어떤 정책에 대한 값인지 식별이 어려움: 할인율이나 정책이 바뀔 때마다 코드를 뒤져서 숫자를 찾아야 함
- 🚨 문제 시나리오: 기획자가 "키보드 할인율을 10%에서 12%로 올려주세요"라고 했을때, 할인율, 수량, 포인트 등 관련없는 정책의 코드를 건드려서 버그가 발생하는 상황

🔧 개선 후 효과

// constants/shopPolicy.js에서 정책 관리
export const DISCOUNT_RATES = {
  PRODUCT: {
    KEYBOARD: 0.1, // 10% 할인
    MOUSE: 0.15, // 15% 할인
    MONITOR_ARM: 0.2, // 20% 할인
  },
  BULK: 0.25, // 25% 대량구매 할인
  TUESDAY: 0.1, // 10% 화요일 할인
};

export const QUANTITY_THRESHOLDS = {
  PRODUCT_DISCOUNT: 10, // 상품별 할인 최소 수량
  BULK_DISCOUNT: 30, // 대량구매 할인 최소 수량
};

정책 변경이 쉬워짐: 할인율 수정할 때 constants 파일 한 곳만 바꾸면 끝
코드 자체가 문서화 역할: QUANTITY_THRESHOLDS.BULK_DISCOUNT를 보면 바로 "대량구매 할인 기준"임을 알 수
있음
실수 방지: 연관 없는 30을 실수로 바꿀 위험이 사라짐

(3) 반복 패턴 함수화

🚨 코드 스멜

간단하지만, 동일한 로직이 빈번히 사용되는 경우: 제품을 찾는 for 루프, format 함수 등이 10개 이상의 위치에서 중복됨
- 🚨 문제 시나리오: 로직 자체가 간단해서 수정이 어렵진 않을것 같아서 유틸로 분리 하지 않고 있었지만, 코드 곳곳에 흩어진 for 루프를 하나하나 찾아서 수정하다가 일부를 놓쳐서 일관성이 깨지는 상황
```
for (let i = 0; i < prodList.length; i++) {
  if (prodList[i].id === productId) {
    return prodList[i];
  }
}
```
중첩된 if-else 구조: 할인 계산 로직이 지나치게 중첩되어 가독성 저하
- 🚨 문제 시나리오: 새로운 상품 "스피커"를 추가하면서 할인율 15%를 적용하려고 했는데, 깊게 중첩된 if-else 구조 때문에 어디에 어떻게 추가해야 할지 헷갈려서 엉뚱한 곳에 코드를 넣어버리는 상황
```
if (curItem.id === 'p1') {
  disc = 0.1;
} else {
  if (curItem.id === 'p2') {
    disc = 0.15;
  } else {
    if (curItem.id === 'p3') {
      disc = 0.2; // 이런 식으로 5단계까지 중첩...
    }
  }
}
```

🔧 개선 후 효과

// utils/productUtils.js - 검색 로직 통합 후 분리
export const findProductById = (productId) =>
  prodList.find((product) => product.id === productId);

// utils/discountUtils.js - 할인 계산 통합 후 분리
export const getProductDiscount = (productId) => {
  const discountMap = {
    [KEYBOARD_ID]: DISCOUNT_RATES.PRODUCT.KEYBOARD,
    [MOUSE_ID]: DISCOUNT_RATES.PRODUCT.MOUSE,
    [MONITOR_ARM_ID]: DISCOUNT_RATES.PRODUCT.MONITOR_ARM,
  };
  return discountMap[productId] || 0;
};

유틸 분리: 검색 방식 변경 시 한 곳만 수정하면 모든 곳에 적용
기존 로직 가독성 개선: 중첩된 if-else를 Map 기반 객체로 평면화하여 코드 흐름 명확화

(4) 컴포넌트 분리

🚨 코드 스멜

거대한 HTML 문자열 덩어리: main 함수 안에서 모든 UI 구조가 하나의 거대한 innerHTML로 처리됨
- 🚨 문제 시나리오: 헤더의 "Shopping Cart" 텍스트를 "장바구니"로 바꾸려고 했는데, main 함수 안에서 어디가 헤더 부분인지 찾느라 10분을 헤매고, 다른 텍스트까지 바꿔버리는 상황
```
function main() {
  root.innerHTML = `
    <div class="mb-8">
      <h1>🛒 Hanghae Online Store</h1>
      // ... 200줄...
      <div class="grid grid-cols-1">
        // ... 200줄...
      </div>
    </div>
  `;
}
```
UI 로직과 비즈니스 로직의 완전한 혼재: DOM 조작 코드 사이사이에 할인 계산이나 재고 관리 로직이 뒤섞여 있음
- 🚨 문제 시나리오: 장바구니에서 상품을 제거하는 기능을 테스트하려고 했는데, DOM 조작과 비즈니스 로직이 하나의 함수에 뒤섞여 있어서 "재고 복구" 로직만 따로 테스트할 방법이 없는 상황
재사용 불가능한 UI 조각들: 같은 스타일의 버튼이나 입력창이 여러 곳에서 필요한데 매번 처음부터 작성해야 함
- 🚨 문제 시나리오: 관리자 페이지에서도 같은 디자인의 버튼을 사용하고 싶었는데, main.js에 하드코딩되어 있어서 CSS 클래스 이름과 구조를 일일이 복사해서 또 다른 중복 코드를 만들어야 하는 상황

🔧 개선 후 효과

// components/Header.js - 헤더 컴포넌트
export function createHeader({ itemCount = 0 }) {
  // 컨테이너 생성
  const container = document.createElement('div');
  container.className = 'mb-8';
  // 컨테이너에 들어갈 요소 조립
  container.innerHTML = `
    <h1 class="text-xs font-medium tracking-extra-wide uppercase mb-2">🛒 Hanghae Online Store</h1>
    <div class="text-5xl tracking-tight leading-none">Shopping Cart</div>
    <p id="item-count" class="text-sm text-gray-500 font-normal mt-3">🛍️ ${itemCount} items in cart</p>
  `;
  // 컨테이너 리턴(HTML 요소)
  return container;
}

규칙적인 컴포넌트 사용방법 설계:
- create*() 함수로 통일: 모든 컴포넌트는 DOM 요소를 반환하는 함수로 구성
- 파라미터로 데이터 주입: { itemCount } 형태로 필요한 데이터만 받아서 처리
- setupEventListeners 메서드: 이벤트 바인딩을 별도 메서드로 분리하여 생명주기 관리
재사용성 확보: Header, CartTotal 등을 다른 페이지에서도 바로 가져다 쓸 수 있음
테스트 용이성: 각 컴포넌트를 독립적으로 테스트 가능하며, 비즈니스 로직과 분리되어 UI 테스트 간편함
React 마이그레이션 준비: 이미 컴포넌트 기반으로 분리되어 있어서 React 컴포넌트로 변환 시 구조적 변경 최소화

(5) 거대한 함수 분해

🚨 코드 스멜

만능 함수(?): 하나의 함수가 6가지 다른 일을 처리하는 엄청난 상황
- 🚨 문제 시나리오: 포인트 계산에서 버그가 발생했는데, handleCalculateCartStuff()의 로직 중 어느 부분에서 문제가 생겼는지 찾지 못하는 하는 상황
```
function handleCalculateCartStuff() {
  // 1. 장바구니 아이템별 합계 계산
  // 2. 할인율 계산
  // 3. 화요일 특가 처리
  // 4. DOM 업데이트
  // 5. 재고 정보 업데이트
  // 6. 포인트 계산
}
```
테스트 불가능한 구조: 할인 계산 로직만 테스트하고 싶은데 DOM 조작까지 함께 실행됨
- 🚨 문제 시나리오: "대량구매 할인이 제대로 계산되나?" 간단한 테스트를 작성하려고 했는데, 함수 하나 호출하면 DOM 요소 찾기, 포인트 계산, 재고 업데이트까지 모든 게 실행되어서 테스트 환경 구성이 지옥이 되는 상황
불필요한 전체 재계산: 상품 하나만 추가해도 전체 장바구니를 처음부터 다시 계산하는 비효율
- 🚨 문제 시나리오: 장바구니에 상품 종류가 50개가 있는 상태에서 1개만 추가했는데, 기존 9개까지 다시 계산하느라 화면이 버벅거리고 사용자가 "이 사이트 왜 이렇게 느려?"라고 불평하는 상황

🔧 개선 후 효과
거대한 함수를 완전히 제거하고 각 이벤트별 전용 핸들러로 분산

// 장바구니 추가 시: 추가 로직만 수행
export function handleAddToCart() {
  const result = addItemToCart(selectedProductId, currentCartQuantity);
  if (!result.success) {
    alert(result.error);
    return;
  }

  // 필요한 업데이트만 수행
  updateSingleCartItem(result);
  const summary = calculateCartSummary(cartItems); // 현재 상태만 계산
  updateUIAfterCartChange(summary);
  handleStockInfoUpdate();
}

// 수량 변경 시: 수량 변경 로직만 수행
export function handleQuantityChange(productId, change) {
  const result = updateCartItemQuantity(productId, change, currentQty);

  // 변경된 아이템만 업데이트
  updateSingleCartItem(result);
  const summary = calculateCartSummary(cartItems); // 현재 상태만 계산
  updateUIAfterCartChange(summary);
}

디버깅 효율성 극대화: 포인트 계산 문제가 생기면 doRenderBonusPoints() 함수만 확인하면 끝. 문제 발생 지점을 단계별로 빠르게 특정 가능
테스트 접근성 개선: 각 함수를 최대한 분리하여 Mock 없이도 단위 테스트 작성 가능. "할인 계산만" 테스트하고 싶으면 calculateDiscounts() 함수에 값만 넣어보면 됨
성능 최적화 가능: 필요한 부분만 재계산하도록 선택적 호출 가능. 재고만 바뀌었으면 DOM 업데이트는 건너뛸 수 있음
코드 가독성 향상: 각 함수의 이름만 봐도 "아, 이건 할인만 계산하는구나" 바로 알 수 있어서 코드 리뷰나 유지보수가 훨씬 편해짐

(6) 전역 변수 제거

🚨 코드 스멜

어디서 쓰는지 찾는데만 한세월인 전역변수들: 변수 선언은 맨 위, 초기화는 중간, 사용은 맨 아래에 있어서 흐름 파악 불가능
- 🚨 문제 시나리오: 포인트 계산이 갑자기 0p로 나오는 버그가 발생했는데, totalAmt 전역 변수를 어느 함수에서 언제 0으로 초기화했는지 찾느라 800줄 코드를 다 뒤져야 하는 상황
```
// 파일 곳곳에 흩어진 8개의 전역 변수들
let bonusPts = 0;
let stockInfo;
let itemCnt = 0;
let lastSel = null;
let sel;
let totalAmt = 0;
let cartDisp;
let sum;
```
함수 간 데이터 공유 가능성: 함수가 어떤 전역 변수에 의존하는지 함수 시그니처만 봐서는 전혀 알 수 없음
- 🚨 문제 시나리오: doRenderBonusPoints() 함수가 내부에서 totalAmt 전역 변수를 사용하는데, 이 함수를 호출하는 다른 개발자는 그걸 모르고 totalAmt를 초기화하지 않아서 포인트가 엉뚱하게 계산되는 상황

🔧 개선 후 효과

디버깅 범위 축소: 문제가 발생했을 때 해당 함수 내부의 변수만 확인하면 되므로 디버깅 시간이 단축됨

(7) 최종 main.js 정리

🚨 코드 스멜

여전히 복잡한 main() 함수: 컴포넌트 분리와 함수 분해는 했지만, main() 함수가 여전히 여러 역할을 담당하고 있는 상황

🔧 개선 후 효과

function main() {
  // 1. 컴포넌트 초기화 (명확한 책임 분리)
  const header = createHeader({ itemCount: 0 });
  const productSelector = createProductSelector();
  const cartDisplay = createCartDisplay();
  const orderSummary = createOrderSummary({...});

  // 2. 컴포넌트 조립 (단순한 DOM 구성)
  leftColumn.appendChild(productSelector);
  rightColumn.appendChild(orderSummary);

  // 3. 리스너 등록 (각 컴포넌트 자체 관리)
  productSelector.setupEventListeners();
  cartDisplay.setupEventListeners();

  // 4. 서비스 시작 (외부 의존성 주입)
  startLightningSale();
}

단일 책임으로 축소: 최종적으로 800줄 → 59줄로 줄어들면서 main()은 "컴포넌트 조립과 초기화"만 담당
가독성 극대화: 메인 코드만 보면 컴포넌트 초기화 → 조립 → 리스너 등록 → 서비스 시작의 명확한 4단계 흐름을 명확하게 파악할 수 있음

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

작업 순서에 대한 아쉬움

작업 순서를 잘못 정했다는 아쉬움이 있습니다. 컴포넌트 분리를 너무 일찍 했고, 전역 변수 제거를 뒤로 미룬다면 더 수월한 리팩토링을 진행할 수 있을 것 같습니다.

실제 진행한 순서:

var → let/const 치환
매직넘버 상수화
반복 패턴 함수화
컴포넌트 분리
거대한 함수 분해
전역 변수 제거
최종 main.js 정리

다시 한다면 진행할 순서:

var → let/const 치환
전역 변수 제거 ← 앞으로 당기기
매직넘버 상수화
반복 패턴 함수화
거대한 함수 분해
컴포넌트 분리 ← 뒤로 미루기
최종 main.js 정리

핵심 문제점:

컴포넌트 분리를 너무 일찍 했음: main 함수의 코드를 줄이려고 서둘러 컴포넌트로 분리했더니, 비즈니스 로직까지 함께 흩어져서 오히려 나중에 함수 분해 작업을 더 어렵게 만들었습니다.
전역 변수 제거를 뒤로 미룬 실수: 나중에 해도 되겠거니 가볍게 생각했지만, 막상 이 작업을 진행해보니 다른 모든 함수들과 연결되어 있어서 일찍 할수록 편한 작업이었음을 깨달았습니다.
단계별 완료를 확실하게 하지 못함: 각 단계를 완벽하게 끝내지 않고 다음 단계로 넘어가다 보니, 나중에 이전 단계로 돌아가서 수정하는 일이 반복됐다. 이 과정에서 작업 순서가 뒤섞이고 효율성이 떨어졌습니다.

완성도에 대한 아쉬움

주어진 과제 수행 시간동안 더 많은 것을 하지 못했던점이 가장 아쉽습니다. 현재 리팩토링 결과물은 여전히 함수 분리, 변수 네이밍, 구조적 개선이 부족한 상태입니다.

직장 업무와 병행하다 보니 과제 분량이 생각보다 많게 느껴졌고, AI 토큰이 떨어져가면서 혼자 해결해야 하는 부분이 늘어나면서 심리적 부담도 컸던것 같습니다. 시간에 쫓기다 보니 코딩에 대한 의지와 체력이 점점 떨어졌지만, 그럼에도 이 과제는 정말 몰입할 수 있었던 주제였습니다.

기회가 있다면 충분한 시간을 두고 더 꼼꼼하게 리팩토링을 진행해보고 싶습니다.

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

위 '과제를 다시 해보면 더 잘 할 수 있었겠다 아쉬운 점이 있다면 무엇인가요?' 항목에서 순서에 대한 아쉬움이 있다고 언급했습니다.
실제 진행한 순서:

var → let/const 치환
매직넘버 상수화
반복 패턴 함수화
컴포넌트 분리
거대한 함수 분해
전역 변수 제거
최종 main.js 정리

다시 한다면 진행할 순서:

var → let/const 치환
전역 변수 제거 ← 앞으로 당기기
매직넘버 상수화
반복 패턴 함수화
거대한 함수 분해
컴포넌트 분리 ← 뒤로 미루기
최종 main.js 정리

저의 실제 작업 진행한 순서에 비해 개선한 순서는 얼마나 적절해 보이는지, 코치님이라면 어떤 순서로 작업을 진행하셨을지 궁금합니다.

- productId - shopPolicy

- getProductDiscount - getBulkBonus

- getQuantityFromElement - formatPrice - createPriceDisplay

- getter/setter/getAll 연결 - products 데이터와 통합

- subTot, totalAmt -> total, originalTotal

- 불필요한 핸들러 분리 - 컴포넌트 이벤트는 각 컴포넌트로 이동 - 카트 핸들러 분리 - 프로덕트 옵션 컴포넌트화

- 간결한 로직이지만 가독성을 위해 분리한 유틸들

tomatopickles404 · 2025-08-01T03:04:51Z

호호 유열님 이번주도 고생하셨습니다~ 글이 유열님처럼 차분하고 논리적이네요 ㅎㅎ 공감되는 부분도 많네요!!

ckdwns9121 · 2025-08-01T04:07:51Z

유열님 고생하셧슴니다~!

unseoJang

안녕하세요 유열님~
2-1 과제 진행하시느라 수고하셨습니다.
전체적인 코드를 살펴보니 UI분리도 잘해주시고 상수화도 잘 해주셨네요
중간 중간 아쉬운 부분들은 if문이 2,3개 이상 나오는 부분들과 관심사 분리가 안되어있는 부분들이 있겟네요
다음번에 복습을 진행한다면 스터디고 과제이니 최대한 함수들을 잘게 쪼개어 보는 연습을 해보면 역할이 많은 함수가 이렇게 까지 쪼개질수 있구나를 깨달을수 있을 것같아요
그 부분이 잘되어있는게 지수님 코드니 지수님 코드 구경 한번 해보는 걸 추천드립니다.
이번 주 과제도 너무 고생 많으셨고 다음 주차 과제도 화이팅해봅시다.

수고 많으셨어요~

unseoJang · 2025-08-01T03:54:55Z

+  };
+
+  // Manual에 setupEventListeners 메서드 추가
+  container.setupEventListeners = function ({ onClose } = {}) {


이 부분 돔과 로직을 분리해서 사용하면 재활용성이 커지지 않을까 싶어요

export function createManual() { const container = document.createElement('div'); container.className = '...'; const column = createManualColumn(); container.appendChild(column); const closeManual = () => { container.classList.add('hidden'); container.querySelector('.transform')?.classList.add('translate-x-full'); }; function setupEventListeners({ onClose } = {}) { container.addEventListener('click', (e) => { if (e.target === container) { closeManual(); onClose?.(); } }); container.querySelector('#manual-close-button')?.addEventListener('click', () => { closeManual(); onClose?.(); }); } return { el: container, setupEventListeners, }; }

const manual = createManual(); document.body.appendChild(manual.el); manual.setupEventListeners({ onClose: () => console.log('닫힘') });

이런식으로요!

unseoJang · 2025-08-01T04:02:11Z

+    // 개별 장바구니 항목 표시
+    for (let i = 0; i < cartItems.length; i++) {
+      const curItem = getProduct(cartItems[i].id);
+      const qtyElem = cartItems[i].querySelector('.quantity-number');


qtyElem는 quantityElement 의 줄임말이죠?

unseoJang · 2025-08-01T04:04:10Z

util함수들을 한곳에 잘 모아두셨네요
util성 성격의 함수들이니 전체적으로 좀 더 주석을 달았으면 가독성과 역할이 구분되어 더 좋았을 것같아요

unseoJang · 2025-08-01T04:05:34Z

계산 util 함수들 따로 모아서 정리한 것들 좋네요

unseoJang · 2025-08-01T04:07:23Z

+) {
+
+  // 화요일 여부를 파라미터로 받거나 현재 날짜로 체크
+  const isTuesdayActual =


const isTuesdayActual = isTuesday ?? new Date().getDay() === 2;

해당코드 병합연산자로 이렇게 줄일수 있을 것같아요

unseoJang · 2025-08-01T04:16:22Z

+import { TIMER_DELAYS, DISCOUNT_RATES } from '../constants/shopPolicy.js';
+import { setProduct, getAllProducts } from '../managers/product.js';
+
+export function startSuggestSale(


이부분 로직 분리를 좀 더 해볼수 있을 것 같아요

function trySuggestSale(getSelectedProduct, onUpdate, onPriceUpdate) { const selectedProductId = getSelectedProduct(); if (!selectedProductId) return; const allProducts = getAllProducts(); const suggest = allProducts.find( (p) => p.id !== selectedProductId && p.quantity > 0 && !p.isSuggestSale ); if (!suggest) return; alert(`💝 ${suggest.name}은(는) 어떠세요? 지금 구매하시면 5% 추가 할인!`); const newPrice = Math.round( suggest.price * (1 - DISCOUNT_RATES.SUGGEST) ); setProduct(suggest.id, { price: newPrice, isSuggestSale: true, }); onUpdate(); onPriceUpdate(); }

function startSuggestSaleTimer(getSelectedProduct, onUpdate, onPriceUpdate) { setInterval(() => { trySuggestSale(getSelectedProduct, onUpdate, onPriceUpdate); }, TIMER_DELAYS.SUGGEST.INTERVAL); }

export function startSuggestSale(getSelectedProduct, onUpdate, onPriceUpdate) { const randomDelay = Math.random() * TIMER_DELAYS.SUGGEST.DELAY_MAX; setTimeout(() => { startSuggestSaleTimer(getSelectedProduct, onUpdate, onPriceUpdate); }, randomDelay); }

아무래도 setTimeout, setInterval 이 저렇게 연달아 나오면 의도 분리가 안되어 있다고 느껴질수도 있다고 생각이드네요.
그리고 안에 익명함수로 setTimeout, setInterval 이 진행이 되다 보니 가독성이 떨어지고 디버깅도 어렵고, 재사용도 어려워지고, 이부분도 의도 분리가 안되어있는 느낌도 들구요

unseoJang · 2025-08-01T04:22:28Z

+        let suggest = null;
+
+        const allProducts = getAllProducts();
+        for (let k = 0; k < allProducts.length; k++) {


이 부분 조건이 너무 많다보니 디버깅할떄 어려움이 많을 것 같아요

export function startSuggestSale() { const delay = Math.random() * TIMER_DELAYS.SUGGEST.DELAY_MAX; setTimeout(() => { setInterval(handleSuggestSale, TIMER_DELAYS.SUGGEST.INTERVAL); }, delay); } function handleSuggestSale() { const selectedProductId = getSelectedProduct(); if (!selectedProductId) return; const suggest = findSuggestedProduct(selectedProductId); if (!suggest) return; applySuggestDiscount(suggest); alert(`💝 ${suggest.name}은(는) 어떠세요? 지금 구매하시면 5% 추가 할인!`); onUpdateSelectOptions(); handlePriceUpdate(); } function findSuggestedProduct(selectedId) { return getAllProducts().find( (p) => p.id !== selectedId && p.quantity > 0 && !p.isSuggestSale ); } function applySuggestDiscount(product) { const newPrice = Math.round( product.price * (1 - DISCOUNT_RATES.SUGGEST) ); setProduct(product.id, { price: newPrice, isSuggestSale: true, }); }

이런식으로 나눠주면 가독성도 좋고 디버깅할떄도 편리하지 않을까 싶어요

k-sang-soo · 2025-08-01T08:45:14Z

고생하셨습니다 유열님!
저는 진행순서를 오히려 컴포넌트 분기를 먼저 했어야 됐나 싶긴 했었는데ㅋㅋㅋㅋㅋ
근데 어차피 UI 로직이랑 비즈니스 로직이랑 합쳐져 있어서 이러나 저러나 어려울 것 같긴하네요

Yuyeol · 2025-08-02T03:26:21Z

고생하셨습니다 유열님! 저는 진행순서를 오히려 컴포넌트 분기를 먼저 했어야 됐나 싶긴 했었는데ㅋㅋㅋㅋㅋ 근데 어차피 UI 로직이랑 비즈니스 로직이랑 합쳐져 있어서 이러나 저러나 어려울 것 같긴하네요

생각없이 전역변수 이동을 나중으로 미룬게 저의 최대 실수였습니다...ㅋㅋ

Yuyeol · 2025-08-02T03:27:38Z

안녕하세요 유열님~ 2-1 과제 진행하시느라 수고하셨습니다. 전체적인 코드를 살펴보니 UI분리도 잘해주시고 상수화도 잘 해주셨네요 중간 중간 아쉬운 부분들은 if문이 2,3개 이상 나오는 부분들과 관심사 분리가 안되어있는 부분들이 있겟네요 다음번에 복습을 진행한다면 스터디고 과제이니 최대한 함수들을 잘게 쪼개어 보는 연습을 해보면 역할이 많은 함수가 이렇게 까지 쪼개질수 있구나를 깨달을수 있을 것같아요 그 부분이 잘되어있는게 지수님 코드니 지수님 코드 구경 한번 해보는 걸 추천드립니다. 이번 주 과제도 너무 고생 많으셨고 다음 주차 과제도 화이팅해봅시다.

수고 많으셨어요~

무엇보다 회사 업무치고 하느라 시간이 많이 부족했던 주였네요 더 꼼꼼하게 하고싶었는데 아쉬움이 많이 남지만 어쩔수없죠...ㅎㅎ

Yuyeol added 10 commits July 28, 2025 15:13

chore: setting lint

547831b

refac: var to let, const 치환

a989447

refac: 상수 분리

b795533

- productId - shopPolicy

refac: 변수 초기화, 재할당 관련 정리

a38fbf0

refac: findProductById 반복 로직 함수 분리

c62fde9

refac: 중첩된 if문에 대한 간소화, 함수분리

b57c388

- getProductDiscount - getBulkBonus

refac: 계산로직 함수화

6a46647

- getQuantityFromElement - formatPrice - createPriceDisplay

docs: 반복로직 정리

fb7e316

refac: 컴포넌트 포맷에 따라 가능한 컴포넌트 1차 분리

ea3fe10

refac: OrderSummary 컴포넌트 세부 분리

67a1e4c

Yuyeol force-pushed the main branch from f01839c to 67a1e4c Compare July 28, 2025 10:42

Yuyeol added 15 commits July 28, 2025 20:14

refac: CartDisplay 컴포넌트 세부 분리

479ef57

refac: Manual 컴포넌트 세부 분리

8bb1f2f

refac: PointDisplay 컴포넌트 형태 통일

e4a1c44

refac: 이벤트 등록방식 통일

7c1cda5

refac: 세일 레이어 분리

9bd1d76

refac: 불필요한 props, 주석 제거

00e6032

refac: product getter 적용

d5bd825

refac: format함수 통일

a59f6b9

refac: productManager 네이밍 변경

722c922

refac: productManager

fbd948d

- getter/setter/getAll 연결 - products 데이터와 통합

refac: selectedProduct 매니저 분리

36917ed

refac: 혼동되는 변수명 정리

8ef9ad3

- subTot, totalAmt -> total, originalTotal

refac: main 코드 정리

deb160e

- 불필요한 핸들러 분리 - 컴포넌트 이벤트는 각 컴포넌트로 이동 - 카트 핸들러 분리 - 프로덕트 옵션 컴포넌트화

fix: ci 에러 수정

3b856e5

chore: react init

f15b789

Yuyeol force-pushed the main branch from 2a68055 to f15b789 Compare July 30, 2025 06:08

Yuyeol added 3 commits July 30, 2025 15:18

[react] feat: 상수 추가

112500a

refac: 전역변수 로컬화

744f7bc

refac: handleCalculateCartStuff 함수 분리

630d8e4

Yuyeol added 14 commits July 30, 2025 17:55

refac: util 분리 - cart

2f5c84d

feat: cartContext - 리듀서 구성, ADD_ITEM 추가

80731e0

feat: cartContext - ADJST_QUANTITY, REMOVE_ITEM 구현

409db0a

feat: OrderSummary - cart값 맵핑

173b8e0

feat: OrderSummary - 상세 할인내역, subTotal 맵핑 수정

4d781e0

feat: discount util 분리

00c027f

feat: OrderSummary - 포인트 ui 맵핑

ba791c5

feat: 번개 sale 알림 적용, AddToCartForm의 옵션 하드코딩 제거

113db1e

feat: 번개세일 가격 적용

3202706

feat: 제안세일 가격 적용, 할인박스 할인율 관련 수정

ace053b

feat: header - 수량 표시

296c6fc

feat: cart - 재고 부족 관련 알림 처리

2a4575f

feat: 썸네일 style 수정

d293f82

chore: ci - deploy 제거

ed7b5e7

Yuyeol force-pushed the main branch from e34359f to ed7b5e7 Compare July 30, 2025 15:25

Yuyeol added 2 commits July 31, 2025 00:55

refac: util 분리 - uiUpdate

ae6052f

refac: global util 분리

9d1f281

- 간결한 로직이지만 가독성을 위해 분리한 유틸들

Yuyeol force-pushed the main branch from cef69d7 to ed7b5e7 Compare July 31, 2025 13:25

Yuyeol added 6 commits July 31, 2025 22:38

refac: AddToCartForm - 자녀 컴포넌트 분리

1729232

fix: 세일 알림 시 장바구니, 셀렉트박스 동시 업데이트가 되지 않는 이슈 수정

2f2713b

refac: productOption - 선언적 로직 추가

99f7282

refac: CartItem - 컴포넌트 분리

d5a7616

refac: CartItem - PriceDisplay 플래그 추가, 분기처리

9a97aac

refac: OrderSummary - 컴포넌트 분리

5e5b3ca

unseoJang reviewed Aug 1, 2025

View reviewed changes

Navigation Menu

Uh oh!

[8팀 정유열] Chapter 2-1. 클린코드와 리팩토링 - #11

[8팀 정유열] Chapter 2-1. 클린코드와 리팩토링#11
Yuyeol wants to merge 57 commits into
hanghae-plus:mainfrom
Yuyeol:main

Yuyeol commented Jul 27, 2025 •

edited

Loading

Uh oh!

tomatopickles404 commented Aug 1, 2025

Uh oh!

ckdwns9121 commented Aug 1, 2025

Uh oh!

unseoJang left a comment

Uh oh!

unseoJang Aug 1, 2025

Uh oh!

unseoJang Aug 1, 2025

Uh oh!

unseoJang Aug 1, 2025

Uh oh!

unseoJang Aug 1, 2025

Uh oh!

unseoJang Aug 1, 2025

Uh oh!

unseoJang Aug 1, 2025

Uh oh!

unseoJang Aug 1, 2025

Uh oh!

k-sang-soo commented Aug 1, 2025

Uh oh!

Yuyeol commented Aug 2, 2025

Uh oh!

Yuyeol commented Aug 2, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Conversation

Yuyeol commented Jul 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

과제 체크포인트

배포링크

기본과제

심화과제

과제 셀프회고

이슈

과제를 하면서 내가 제일 신경 쓴 부분은 무엇인가요?

작업순서를 정하고 그에 따라 작업하기

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

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

Uh oh!

tomatopickles404 commented Aug 1, 2025

Uh oh!

ckdwns9121 commented Aug 1, 2025

Uh oh!

unseoJang left a comment

Choose a reason for hiding this comment

Uh oh!

unseoJang Aug 1, 2025

Choose a reason for hiding this comment

Uh oh!

unseoJang Aug 1, 2025

Choose a reason for hiding this comment

Uh oh!

unseoJang Aug 1, 2025

Choose a reason for hiding this comment

Uh oh!

unseoJang Aug 1, 2025

Choose a reason for hiding this comment

Uh oh!

unseoJang Aug 1, 2025

Choose a reason for hiding this comment

Uh oh!

unseoJang Aug 1, 2025

Choose a reason for hiding this comment

Uh oh!

unseoJang Aug 1, 2025

Choose a reason for hiding this comment

Uh oh!

k-sang-soo commented Aug 1, 2025

Uh oh!

Yuyeol commented Aug 2, 2025

Uh oh!

Yuyeol commented Aug 2, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Yuyeol commented Jul 27, 2025 •

edited

Loading