Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[15팀 김재환] [Chapter 2-1] 클린코드와 리팩토링 #37

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

kJshine
Copy link

@kJshine kJshine commented Jan 9, 2025

과제 체크포인트

기본과제

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

심화과제

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

과제 셀프회고

  • 이번 과제는 투자할 수 있는 시간이 너무 적어서 너무 아쉬웠습니다..... 최대한 진행을 해보려고 했지만 심화과제쪽은 아예 작업을 못해서 너무 아쉬웠습니다.

과제에서 좋았던 부분

  • 이전 회사에서 코드 컨벤션을 정해서 진행해본 기회가 없었어서 굉장히 유익힌 시간이였다
  • 다양한 의견들을 모아 보다 모두가 납득할 수 있는 좋은 코드 규칙을 만들어 내는 과정이 좋았습니다.

과제를 하면서 새롭게 알게된 점

  • 더티 코드를 리팩토링 한다는 것이 새로 작성하는 방식이 아닌 기존 코드를 살리는 것이라는 걸 알게되었습니다.

과제를 진행하면서 아직 애매하게 잘 모르겠다 하는 점, 혹은 뭔가 잘 안되서 아쉬운 것들

  • 어디서부터 손을 봐야할지 막막한 느낌이였습니다...
  • 단위 별로 리팩토링을 하는 것을 의식했지만... 쉽지 않다는 것을 느꼈습니다.

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

  • 전역변수로 오염된 코드를 전역변수를 사용하지 않도록 바꾸는 좋은 방법이 무엇이 있을까요?

Comment on lines +21 to +31
function main() {
calcCart();

renderCart();

renderProductSelectOptions();

alertLuckySale();

alertSuggestProduct();
}
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 +1 to +8
export const utils = () => {
// 랜덤 인덱스 값
const randomIndex = (length) => Math.floor(Math.random() * length);

return {
randomIndex,
};
};
Copy link

Choose a reason for hiding this comment

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

유틸을 객체로 관리하는 이유가 혹시 있을까요!

randomIndex 자체로 function을 export 하면 어떨까 해서요!

Comment on lines 21 to +31
function main() {
prodList=[
{id: 'p1', name: '상품1', val: 10000, q: 50 },
{id: 'p2', name: '상품2', val: 20000, q: 30 },
{id: 'p3', name: '상품3', val: 30000, q: 20 },
{id: 'p4', name: '상품4', val: 15000, q: 0 },
{id: 'p5', name: '상품5', val: 25000, q: 10 }
];
var root=document.getElementById('app');
let cont=document.createElement('div');
var wrap=document.createElement('div');
let hTxt=document.createElement('h1');
cartDisp=document.createElement('div');
sum=document.createElement('div');
sel=document.createElement('select');
addBtn=document.createElement('button');
stockInfo=document.createElement('div');
cartDisp.id='cart-items';
sum.id='cart-total';
sel.id='product-select';
addBtn.id='add-to-cart';
stockInfo.id='stock-status';
cont.className='bg-gray-100 p-8';
wrap.className='max-w-md mx-auto bg-white rounded-xl shadow-md overflow-hidden md:max-w-2xl p-8';
hTxt.className='text-2xl font-bold mb-4';
sum.className='text-xl font-bold my-4';
sel.className='border rounded p-2 mr-2';
addBtn.className='bg-blue-500 text-white px-4 py-2 rounded';
stockInfo.className='text-sm text-gray-500 mt-2';
hTxt.textContent='장바구니';
addBtn.textContent='추가';
updateSelOpts();
wrap.appendChild(hTxt);
wrap.appendChild(cartDisp);
wrap.appendChild(sum);
wrap.appendChild(sel);
wrap.appendChild(addBtn);
wrap.appendChild(stockInfo);
cont.appendChild(wrap);
root.appendChild(cont);
calcCart();

renderCart();

renderProductSelectOptions();

alertLuckySale();

alertSuggestProduct();
}
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 211 to 308
itemToAdd.q--;
const newItem = document.createElement('div');
newItem.id = itemToAdd.id;
newItem.className = 'flex justify-between items-center mb-2';
newItem.innerHTML = `
<span>${itemToAdd.name} - ${itemToAdd.price}원 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>
`;
ElementCartItems.appendChild(newItem);
itemToAdd.stock--;
}

calcCart();
lastSel=selItem;
lastAddCartProductId = addProductId;
}
});
cartDisp.addEventListener('click', function (event) {
var tgt=event.target;
if(tgt.classList.contains('quantity-change') || tgt.classList.contains('remove-item')) {
var prodId=tgt.dataset.productId;
var itemElem=document.getElementById(prodId);
var prod=prodList.find(function (p) { return p.id === prodId; });
if(tgt.classList.contains('quantity-change')) {
var qtyChange=parseInt(tgt.dataset.change);
var newQty=parseInt(itemElem.querySelector('span').textContent.split('x ')[1]) + qtyChange;
if(newQty > 0 && newQty <= prod.q + parseInt(itemElem.querySelector('span').textContent.split('x ')[1])) {
itemElem.querySelector('span').textContent=itemElem.querySelector('span').textContent.split('x ')[0] + 'x ' + newQty;
prod.q -= qtyChange;
} else if(newQty <= 0) {
itemElem.remove();
prod.q -= qtyChange;

ElementCartItems.addEventListener('click', function (event) {
const target = event.target;

if (
target.classList.contains('quantity-change') ||
target.classList.contains('remove-item')
) {
const productId = target.dataset.productId;
const ElementCartItem = document.getElementById(productId);
const product = productList.find(function (product) {
return product.id === productId;
});

if (target.classList.contains('quantity-change')) {
const qtyChange = parseInt(target.dataset.change);
const newQty =
parseInt(
ElementCartItem.querySelector('span').textContent.split('x ')[1]
) + qtyChange;
if (
newQty > 0 &&
newQty <=
product.stock +
parseInt(
ElementCartItem.querySelector('span').textContent.split('x ')[1]
)
) {
ElementCartItem.querySelector('span').textContent =
ElementCartItem.querySelector('span').textContent.split('x ')[0] +
'x ' +
newQty;
product.stock -= qtyChange;
} else if (newQty <= 0) {
ElementCartItem.remove();
product.stock -= qtyChange;
} else {
alert('재고가 부족합니다.');
alert(OUT_OF_STOCK_MSG);
}
} else if(tgt.classList.contains('remove-item')) {
var remQty=parseInt(itemElem.querySelector('span').textContent.split('x ')[1]);
prod.q += remQty;
itemElem.remove();
} else if (target.classList.contains('remove-item')) {
const remQuantity = parseInt(
ElementCartItem.querySelector('span').textContent.split('x ')[1]
);
product.stock += remQuantity;
ElementCartItem.remove();
}
calcCart();
}
}); 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.

이벤트 위임 방식으로 한 것 너무 흥미롭네요!

const randomIndex = (length: number) => Math.floor(Math.random() * length);

return {
randomIndex,

Choose a reason for hiding this comment

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

오호.. 바로 return randomIndex 안하시구, 요러케 하신 이유가 있나유?

Copy link

@hty0525 hty0525 left a comment

Choose a reason for hiding this comment

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

깔끔한 코드 잘 봤습니다!!
할인률이나 상수화 해도 되는 값들을 했으면 어땟나 싶어요!
이번 주도 고생하셨습니다!

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.

4 participants