ckdwns9121 님의 상세페이지 ＞ [8팀 박창준] Chapter 2-1. 클린코드와 리팩토링

[8팀 박창준] Chapter 2-1. 클린코드와 리팩토링

by ckdwns91212025. 7. 27.

과제 체크포인트

배포링크

배포링크

기본과제

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

심화과제

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

과제 셀프회고

우선 과제 양이 생각보다 많아서 놀랐습니다. 다시 1주차 과제로 돌아간 느낌이 들기도 했네요... 하지만 제가 평소에 레거시 코드를 리팩토링하고 클린한 구조로 개선하는 것에 관심이 많다 보니, 이번 과제는 꽤 즐겁게 몰입해서 할 수 있었습니다.

다만 코드를 확인해보니 아 이거 쉽지 않겠구나.. 싶었습니다. AI 시대에 있어서 AI가 코드 생산은 잘 해주지만, 이런 복잡하고 엉켜버린 레거시 코드를 깔끔한 아키텍쳐로 재정비 하는 일은 AI가 할 수 있을까? 이건 사람의 영역 아닌가? 하는 궁금증도 들었고 거의 재건축 수준의 작업이 될 것 같았습니다.

도대체 어디서부터 손을 대야하지? 하는 막막함이 가장 컸는데 AI에게 어떤 순서대로 리팩토링을 하면 좋을지 조언을 받았고, 그걸 기반으로 제 나름의 리팩토링 플로우를 정립해봤습니다.

변수 선언 개선 (var 제거, let to const) → 중복 변수 제거 → 매직넘버 상수화 → 관심사 분리 → 함수 분리 및 단일 책임 적용 → 모듈 화 및 구조 개선 → 고급 리팩토링(디자인패턴 적용이라던지, 새로운 아키텍쳐 적용)

이런 흐름으로 작업을 진행했습니다.

그리고 도은님이 추천해주신 Cursor Rules를 기반으로 클린코드 룰도 적용하여서 많은 도움을 받았습니다. 감사합니다.

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

1. 단일 책임 원칙을 지키기 위한 구조 설계

  • 비즈니스 로직, UI 렌더링, 상태 저장, 데이터 변환 등 각각의 역할이 섞이지 않도록 최대한 명확한 경계로 나누는 것을 최우선으로 했습니다.
  • 이를 위해 UI에서 비즈니스 상태를 수정하지 않고, 비즈니스 로직에서 DOM에 직접 접근하지 않는 원칙을 지키면서 코드를 수정했습니다.
  • 비즈니스 로직은 Service단, 상태관리는 Store단, 이벤트 관리는 Lisnter단에서 관리하였습니다.
// Service단에서는 비즈니스 로직을 처리하게 함.

export class CartService {
  addProductToCart(product, quantity = 1) {
    if (!this.validateStock(product, quantity)) {
      return false;
    }

    const { cartItems } = this.cartStore.getState();
    const existingItem = cartItems.find(item => item.id === product.id);

    if (existingItem) {
      existingItem.quantity += quantity;
    } else {
      const newItem = {
        id: product.id,
        name: product.name,
        price: product.price,
        quantity,
      };
      this.cartStore.setState({
        cartItems: [...cartItems, newItem],
      });
    }

    product.quantity -= quantity;
    this.updateCartTotals();
    return true;
  }
}

// Store는 상태와, getter setter만 구현해서 단일 책임을 지게함.
export class CartStore {
  constructor() {
    this.state = {
      cartItems: [],
      totalAmount: 0,
      itemCount: 0,
      discountRate: 0,
      savedAmount: 0,
      lastSelectedProduct: null,
    };
  }

  // 불변성을 유지하는 상태 업데이트
  setState(newState) {
    this.state = { ...this.state, ...newState };
  }

  // 상태 조회
  getState() {
    return this.state;
  }
}

export default CartStore;

2. 순수 함수와 상태 캡슐화

  • 가능한 많은 함수들을 순수 함수로 만들려 했습니다.
  • 외부 상태를 참조하거나 변경하지 않고, 동일한 입력에 대해 동일한 출력을 보장하도록 설계하려고 신경을 썼습니다.
  • 또한 상태는 store로 캡슐화를 진행하였습니다.
// ❌ 부작용이 있는 함수
function calculateDiscount(cartItems) {
  // 외부 상태 참조
  const tuesdayDiscount = isTuesday(); // 외부 함수 호출

  // 외부 상태 변경
  document.querySelector("#discount").textContent = discount;

  return discount;
}

// ✅ 순수 함수
export function calculateCartSummary(cartItems, discountService, cartService) {
  // 모든 의존성을 매개변수로 받음
  const discountResult = discountService.applyAllDiscounts(cartItems, productList);
  const itemCount = cartService.getItemCount();

  // 외부 상태 변경 없음
  return {
    success: true,
    cartItems,
    discountResult,
    itemCount,
  };
}

// ✅ 상태 캡슐화
export class CartService {
  constructor() {
    this.cartStore = new CartStore(); // 상태 관리 캡슐화
  }

  getState() {
    return this.cartStore.getState(); // 상태 접근 제어
  }

  setState(newState) {
    this.cartStore.setState(newState); // 상태 변경 제어
  }
}

3. 서비스 간 의존성 최소화

  • 순환 의존성을 방지하려고 의존성 주입 패턴을 적용하였습니다.
  • 예를들어서 CartService는 DiscountService와 ProductService를 외부에서 주입받도록 설계하여 서비스의 의존성을 최소화 하였습니다.
// ❌ 강한 의존성
class CartService {
  constructor() {
    this.discountService = new DiscountService(); // 직접 생성
    this.productService = new ProductService();
  }
}

// ✅ 의존성 주입
export class CartEventListeners {
  constructor(uiEventBus, cartService, discountService, productService) {
    this.uiEventBus = uiEventBus;
    this.cartService = cartService;
    this.discountService = discountService;
    this.productService = productService;
  }
}

// 메인에서 의존성 주입
const cartService = new CartService();
const discountService = new DiscountService();
const productService = new ProductService();

const cartListeners = new CartEventListeners(uiEventBus, cartService, discountService, productService);

4. Event Bus 아키텍처 도입

이번 과제에서 가장 큰 고민을 했던 부분은 비즈니스 로직과 UI 로직의 분리였습니다. 이 고민을 해결하려고 이벤트 버스(EventBus) 아키텍처를 도입해 모듈 간 결합도를 낮추는 방식으로 접근했는데요, 구조 자체는 느슨한 연결을 만들어 유지보수에 도움이 되었습니다.


    // 장바구니 요약 계산 완료 이벤트 처리
    this.uiEventBus.on(CART_SUMMARY_CALCULATED, data => {
      if (data.success) {
        this.renderCartUI(data.cartItems, data.discountResult, data.itemCount);
      }
    });

    // 장바구니 아이템 스타일 업데이트 이벤트
    this.uiEventBus.on(CART_ITEM_STYLES_UPDATED, data => {
      if (data.success) {
        this.updateCartItemStyles(data.cartItems);
      }
    });

    // 장바구니 가격 업데이트 요청 이벤트 처리
    this.uiEventBus.on(CART_PRICES_UPDATE_REQUESTED, data => {
      if (data.success) {
        this.handleCartPricesUpdate();
      }
    });

    // 헤더 아이템 카운트 업데이트 이벤트
    this.uiEventBus.on(HEADER_ITEM_COUNT_UPDATED, data => {
      if (data.success) {
        this.renderHeaderItemCount(data.itemCount);
      }
    });

하지만 구조적인 한계도 분명히 경험하게 되었습니다. 실제로는 아래와 같은 구조적 한계와 설계 고민이 함께 따랐습니다.

  • 어떤 레이어에 이벤트버스를 위치시킬지?
  • 이벤트 버스를 싱글톤으로 둘지, 모듈마다 주입을 시킬지
  • 특히나 어려웠던 이벤트 키 네이밍…

그렇지만 분명 구조적인 장점이 있었고 저는 두 가지를 뽑아봤습니다.

느슨한 결합

이벤트 버스를 중심으로 모듈 간 직접 호출 없이 메시지를 주고받는 구조로 변경하면서, 모듈 간 의존성을 낮추고 변경에 유연한 구조를 만들 수 있었습니다. 예를 들어 ProductSelector는 CartService를 직접 알지 못하지만, CART_ADD_REQUESTED 이벤트만 발행하면 나머지 처리는 이벤트 흐름 안에서 자연스럽게 이어집니다.

단일 책임

기존에는 비즈니스 로직, 상태 관리, 렌더링 로직이 한 파일에 뒤섞여 있었지만, 이를 CartService, CartStore, CartList, CartEventListeners 등으로 나누어 각 모듈이 하나의 책임만 갖도록 구조화했습니다. 유지보수는 물론, 의도를 파악하기에도 훨씬 쉬워졌습니다.

아래는 위 구조를 시각화 해보았습니다.

graph TB
    subgraph "Loose Coupling (느슨한 결합)"
        A[ProductSelector] -->|CART_ADD_REQUESTED| B[EventBus]
        B -->|handleAddToCart| C[CartEventListeners]
        C -->|addProductToCart| D[CartService]
        D -->|setState| E[CartStore]
        C -->|CART_ITEM_ADDED| B
        B -->|createCartItem| F[CartList]
    end
    
    subgraph "Single Responsibility (단일 책임)"
        G[CartService<br/>비즈니스 로직만]
        H[CartEventListeners<br/>이벤트 처리만]
        I[CartStore<br/>상태 관리만]
        J[CartList<br/>UI 렌더링만]
    end
    
    subgraph "Testability (테스트 가능성)"
        K[Unit Test<br/>CartService]
        L[Unit Test<br/>CartStore]
        M[Integration Test<br/>EventBus]
    end

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

너무 욕심을 많이 내다 보니 아쉬움이 많습니다. 처음부터 컨벤션을 제대로 정하고 폴더구조나 데이터 흐름, UI로직 등을 좀더 고민했더라면 더 좋은 구조가 나왔지 않았을까 생각합니다. 무작정 리팩토링 부터 진행하면서 계획대로 되지 않았고, 그러다보니 구조를 여러번 바꾸고 분리해보면서 중복되는 로직이나 순환 의존성 같은 문제들이 나타났습니다.

그중에서도 가장 아쉬웠던 선택은 이벤트 버스패턴 도입이었습니다. 처음에는 모듈 간 결합도를 낮추고 깔끔한 아키텍처를 만들 수 있을 것이라 기대했지만.. 이벤트 흐름을 추적하기 어렵고, 이벤트 체인이 길어질수록 디버깅 난이도가 급격히 올라가며,특히 네이밍과 이벤트 설계 원칙이 모호해지면서 유지보수성이 떨어지는 문제 등을 경험했습니다. 결국, 옵저버 패턴 + 상태 관리 정도로만 설계해도 이번 과제의 요구사항에는 충분했을 것 같다는 아쉬움이 남습니다.

하지만 반대로, 이번 기회에 이렇게 이벤트 버스 패턴을 직접 적용해보면서 덕분에 이벤트 버스 아키텍처가 어떤 상황에서 효과적이고 어떤 상황에서는 과할 수 있는지를 실제로 겪고 배울 수 있었습니다.

앞으로는 복잡한 구조가 ‘잘 만든 구조’는 아니라는 것, 그리고 경량화된 아키텍처가 오히려 더 유지보수성과 코드의 가독성을 높여준다는 점 을 더 고려해서 설계할 수 있을 것 같습니다 ㅎㅎ

두번째로 아쉬운점은.. 비즈니스 로직과, 유틸, 헬퍼 함수를 깔끔하게 분리한뒤 리액트에서 재사용하게 하고싶었습니다. 하지만 생각보다 잘 안되었네요.. 아마 클래스를 사용해서 비즈니스 로직을 관리하다 보니 리액트에서 재사용하기가 어려웠던 것 같습니다

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

Q1. 특정 서비스에서 아주 일부 기능(1~2개 함수)만 사용하기 위해 전체 서비스를 주입하는 건 과도한 의존일까요?

예를 들어, 저는 OrderService에서 DiscountService의 calculateProductDiscountRate() 함수 하나만 사용하기 위해 전체 DiscountService를 주입했습니다:

export class OrderService {

  constructor(discountService) {
    this.orderStore = new OrderStore();
    this.discountService = discountService;
  }
   
  calculateOrderData(cartItems,productList){
        // 주입받은 discountService를 사용하여 할인 계산
    const discountResult = this.discountService.applyAllDiscounts(cartItems, productList);
   ... 
  }

그리고 실제 사용하는건

const discountResult = this.discountService.applyAllDiscounts(cartItems, productList);

이 한줄입니다.

이처럼 특정 서비스의 일부 기능만 필요할 때, 전체 서비스 객체를 통째로 주입하는 게 맞는지? 단일 책임이나 의존성 최소화 관점에서 보면, 오히려 이 주입이 너무 큰 단위로 이루어졌다고도 느껴졌습니다.

그래서 해당 함수만 따로 유틸로 분리해서 사용하면 어떨까 라는 고민도 했는데 결국 DiscountService(할인 관련된 로직)의 응집도가 깨져버리고 할인 정책에 대한 책임이 이곳저곳 흩어지는 결과를 낳게되는게 아닐까? 라는 고민도 들었습니다..

실무에서는 이런 비슷한 상황이 있으면 어떤 기준으로 설계 결정을 하시는지 궁금합니다.

과제 피드백

고생하셨습니다 창준님. 이런 코드를 개선하는데에서 즐거움을 느끼셨다니 대단하신데요 ㅎㅎ 나름의 흐름을 정의하고 접근 하는 방향은 매우 좋았습니다. 클린 코드의 관점에서도 많은 부분을 고민하셨던 게 문서에 나와있어서 잘 하셨습니다. 과제에 있어서는 좀 더 개선이 필요한 부분이 있지만 충분히 시간적 여유가 있다면 지금의 관점으로 접근하면 개선이 가능할 것 같아요. 잘 하셨습니다!

질문 주신 부분 답변 드려보고 마무리해볼게요.

특정 서비스에서 아주 일부 기능(1~2개 함수)만 사용하기 위해 전체 서비스를 주입하는 건 과도한 의존일까요?

우선 지금의 설계가 나쁘다고는 전혀 생각이 들지는 않는 것 같아요. 회고에서 작성해주신것처럼 서비스간의 의존성을 낮추는 관점, 테스트의 용이성 관점에서 코드 파악이 쉬워지고 응집도도 높아지는 것 같아요. 변경에 유연하기도 하구요. 전혀 과도해보이지는 않아요!

고생하셨고 다음주 과제도 이번 주처럼 잘 진행해보시면 좋겠습니다. 화이팅입니다!