[15팀 차현빈] [Chapter 2-1] 클린코드와 리팩토링

[chb6734]

과제 체크포인트

기본과제

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

심화과제

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

과제 셀프회고

과제에서 좋았던 부분

이번 과제를 진행하면서, 그동안 회사에서 사용되던 변수명 규칙이 애매하고 이해하기 어려운 경우가 많았던 점을 돌아보게 되었습니다. 특히, 최근에 한 함수에 if문이 30개씩 들어가 있는 거대한 함수들 때문에 리팩토링의 필요성을 절실히 느꼈었지만. 함께 일하는 동료들과 어떤 기준으로 규칙을 세우고, 이를 어떻게 적용할지 경험이 부족해 망설였던 부분도 있었습니다.

하지만 이번 과제를 진행하면서 페어팀과 컨벤션을 만들어보고, 그 컨벤션을 최대한 지키려고 노력하는 과정에서 처음부터 완벽할 수는 없더라도 방향을 잡고 지속적으로 개선해 나가면 된다는 확신을 얻을 수 있었습니다.
그리고 그 느낌으로 바로 회사 코드에 배운 내용을 적용해 보려다 보니 이번 주는 과제에 쏟을 시간이 부족했던 점이 너무나도 아쉬웠습니다. 그래도 이번 과제가 방향성을 잡는데 도움을 주고 생각만 하고 있었던 리팩토링을 시작해 볼 수 있게 해줬다는 점에서 너무나 유익했습니다.

리뷰 받고 싶은 내용이나 궁금한 것에 대한 질문

이번 과제에서 중요하게 생각하고 수행하려고 한 부분은

1. 함수는 최대한 인풋과 아웃풋이 있게, UI와 기능을 분리하기
2. 추상화 수준 맞춰보기 였습니다

각각의 부분에서 에메한 부분들이 있었는데

1. main() 함수는 비즈니스 로직보다는 UI를 그리는 로직이라고 생각되어졌습니다. 그래서 addNewCartItem() 같은 함수로 UI를 그리는 부분을 따로 분리해야 할지, 아니면 그대로 main() 함수 안에 둬도 괜찮을지 고민입니다.
   분리하면 코드가 더 모듈화되고 가독성이 좋아질 수 있지만, UI 로직만 따로 빼는 것이 과도한 분리가 되는 것이 아닌가 생각했습니다.
   그렇지만 그대로 두면 main() 함수가 UI와 비즈니스 초기화를 한곳에서 처리하기 때문에 흐름이 명확할 수 있지만, 함수가 길어진다는 단점을 갖게 되는 것 같습니다.
   이런 상황에서 UI를 그리는 로직을 따로 함수로 분리하는 것이 좋은지, 그대로 두는 것이 좋은지에 대한 기준이 궁금합니다.

function main() {
  const root = document.getElementById("app");

  const contents = document.createElement("div");
  contents.className = "bg-gray-100 p-8";
  contents.innerHTML = `
    <div class="max-w-md mx-auto bg-white rounded-xl shadow-md overflow-hidden md:max-w-2xl p-8">
      <h1 class="text-2xl font-bold mb-4">장바구니</h1>
      <div id="${ID_BY_COMPONENT.CART_ID}"></div>
      <div id="${ID_BY_COMPONENT.SUM_ID}" class="text-xl font-bold my-4"></div>
      <select id="${ID_BY_COMPONENT.SELECT_ID}" class="border rounded p-2 mr-2"></select>
      <button id="${ID_BY_COMPONENT.ADD_BTN_ID}" class="bg-blue-500 text-white px-4 py-2 rounded">추가</button>
      <div id="${ID_BY_COMPONENT.STOCK_INFO_ID}" class="text-sm text-gray-500 mt-2"></div>
    </div>
  `;

  root.appendChild(contents);

  updateSelectOption();
  updateCart();

  setLuckySaleAlert();
  setSuggestDiscountedProductAlert();
}

/** 장바구니에 새로운 상품 추가 */
const addNewCartItem = (cart, itemToAdd) => {
  let newItem = document.createElement("div");
  newItem.id = itemToAdd.id;
  newItem.className = "flex justify-between items-center mb-2";
  newItem.innerHTML = `
    <span>${itemToAdd.name} - ${itemToAdd.val}원 x 1</span>
    <div>
      <button class="quantity-change bg-blue-500 text-white px-2 py-1 rounded mr-1" 
              data-product-id="${itemToAdd.id}" data-change="-1">-</button>
      <button class="quantity-change bg-blue-500 text-white px-2 py-1 rounded mr-1" 
              data-product-id="${itemToAdd.id}" data-change="1">+</button>
      <button class="remove-item bg-red-500 text-white px-2 py-1 rounded" 
              data-product-id="${itemToAdd.id}">삭제</button>
    </div>`;
  cart.appendChild(newItem);
  itemToAdd.qty--;
};

2. 아래 updateExistingCartItem()에서 내부 IF문에서 추상화 수준을 맞춰주려면 if문에 있는 코드를 else문에 있는 함수처럼 묶어주는 것이 좋은 방법일지 아니면 현재처럼 풀어쓰는 것이 더 좋은 방법일지 궁금합니다.

/** 장바구니에 존재하는 상품 수량 변경  */
const updateExistingCartItem = (item, itemToAdd) => {
  let newQty =
    parseInt(item.querySelector("span").textContent.split("x ")[1]) + 1;

  if (newQty <= itemToAdd.qty) {
    item.querySelector("span").textContent =
      `${itemToAdd.name} - ${itemToAdd.val}원 x ${newQty}`;
    itemToAdd.qty--;
  } else {
    showOutOfStockAlert();
  }
};

[hty0525]

하나의 state로 관리하는 것 좋아보이는데요?
이렇게 생각하게 된 이유가 있을까요?

[hty0525]

0으로 초기화 하는 것 말고, 전부 더한 값을 바로 할당하는 방법은 어떤가요?

[hty0525]

주석이 깔쌈하게 잘 되는거 너무 보기 좋네요!
특히 jsDoc 쓴거 짱입니다
이번 주 고생하셨습니다!