🚨 코드 스멜

• 전역 변수 선언: 어디서든 접근 가능해서 의도치 않은 변경 위험

  • 🚨 문제 시나리오: 다른 스크립트에서 전역 변수를 선언하거나, 함수 내부에서 실수로 전역 변수를 수정해서 전혀 다른 곳에서 예상치 못한 동작이 발생할 수 있음.

  var prodList;
  var bonusPts = 0;
  ...
  

• var의 재선언이 가능한 특징: 같은 변수를 여러 번 선언하는 패턴이 가능하여 최초 선언인지 아닌지 구분이 어려움

  • 🚨 문제 시나리오: 팀원간 다른 의도로 동일 변수명 선언한 경우 의도치 않은 버그를 일으킬 수 있음.

  function main() {
    var root, header, gridContainer;
    // ... 50줄 후 ...
    var root = document.getElementById('app');
  }
  

• 호이스팅: 전역 변수가 이미 window에 등록되어 있어서 선언 전 출력 시에도 에러가 아닌 undefined 반환

  • 🚨 문제 시나리오: 변수를 선언하기 전에 사용해도 에러가 발생하지 않아서 초기화되지 않은 변수로 계산이 진행되어 NaN이나 예상치 못한 결과가 나오는 버그가 발생할 수 있음.

🔧 개선 후 효과

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

🚨 코드 스멜

• 하드코딩된 매직넘버 산재: 할인율, 수량 기준, 타이머 값들이 의미를 알기 어려운 숫자로 흩어져 있음

  • 🚨 문제 시나리오: 대량구매 기준을 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, // 대량구매 할인 최소 수량
};

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

🚨 코드 스멜

• 간단하지만, 동일한 로직이 빈번히 사용되는 경우: 제품을 찾는 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;
};

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

🚨 코드 스멜

• 거대한 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;
}

1. 규칙적인 컴포넌트 사용방법 설계:

   • create*() 함수로 통일: 모든 컴포넌트는 DOM 요소를 반환하는 함수로 구성
   • 파라미터로 데이터 주입: { itemCount } 형태로 필요한 데이터만 받아서 처리
   • setupEventListeners 메서드: 이벤트 바인딩을 별도 메서드로 분리하여 생명주기 관리

2. 재사용성 확보: Header, CartTotal 등을 다른 페이지에서도 바로 가져다 쓸 수 있음

3. 테스트 용이성: 각 컴포넌트를 독립적으로 테스트 가능하며, 비즈니스 로직과 분리되어 UI 테스트 간편함

4. React 마이그레이션 준비: 이미 컴포넌트 기반으로 분리되어 있어서 React 컴포넌트로 변환 시 구조적 변경 최소화

🚨 코드 스멜

• 만능 함수(?): 하나의 함수가 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);
}

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

🚨 코드 스멜

• 어디서 쓰는지 찾는데만 한세월인 전역변수들: 변수 선언은 맨 위, 초기화는 중간, 사용은 맨 아래에 있어서 흐름 파악 불가능

  • 🚨 문제 시나리오: 포인트 계산이 갑자기 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를 초기화하지 않아서 포인트가 엉뚱하게 계산되는 상황

🔧 개선 후 효과

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

🚨 코드 스멜

• 여전히 복잡한 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();
}

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