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

[13팀 김유진] [Chapter 2-1] 클린코드와 리팩토링 #40

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

Conversation

yireum
Copy link

@yireum yireum commented Jan 9, 2025

과제 체크포인트

기본과제

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

심화과제

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

과제 셀프회고

과제를 할 때 내가 하는 게 맞나 하는 생각은 많이 들었지만 이번엔 정말로 이게 맞나 하는 생각이 많이 들었던 과제같습니다. 그래도 코치님이 체크리스트를 주신 덕에 하나씩 체크하면서 해나갔지만 심화과제에서 시간이 부족해 아쉬움이 남습니다. 좀 더 깔끔하게 수정하고 테스트 코드도 구현해 볼 예정입니다.

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

  • 체크리스트 중에 ‘코드의 각 부분이 테스트 가능하도록 구조화되어 있는가?’ 에 대해서 어떤식으로 개선해야 좋은 건지 감이 잘 안왔습니다..
  • 사실 체크리스트 순서대로 해 나가면서 이게 맞나 하는 생각이 많이 들었던 것 같습니다. 제가 이해한 과제 방향성이 맞는지도 궁금합니다!

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

  • 리팩토링 시에 코드의 각 부분이 테스트 가능하도록 구조화 한다는 게 어떤 의미이고 어떤 식으로 개선해야 하는지 모르겠습니다.
  • 커밋 내역을 한번만 봐주시고.. 제가 구현한 단계 단계가 어느정도 적절한지 말씀 한번 해주세요.. 하면서도 오히려 자신이 없어지는 그런 느낌이어서 힘들었습니다.. ㅜㅜ

yireum added 21 commits January 6, 2025 21:56
Comment on lines +14 to +27
const PROMOTION_CONFIG = {
FLASH_SALE: {
INTERVAL: 30000,
INITIAL_DELAY: 10000,
PROBABILITY: 0.3,
DISCOUNT_RATE: 0.8,
},
RECOMMENDATION: {
INTERVAL: 60000,
INITIAL_DELAY: 20000,
DISCOUNT_RATE: 0.95,
},
};

Copy link

Choose a reason for hiding this comment

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

오,, 완전 세세하게 상수화 대박이군요

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.

기본과제 진짜 역할별로 분리 기깔나게 했네요 와,,,, 할말이 없어요 ㅋㅋㅋㅋㅋ

Comment on lines +10 to +78
useEffect(() => {
// 번개세일
const flashSaleConfig = PROMOTION_CONFIG.FLASH_SALE;
const flashSaleTimer = setTimeout(() => {
const interval = setInterval(() => {
if (Math.random() < flashSaleConfig.PROBABILITY) {
setProductList((prev) => {
const availableProductList = prev.filter((p) => p.stock > 0);
if (availableProductList.length === 0) return prev;

const randomProduct =
availableProductList[
Math.floor(Math.random() * availableProductList.length)
];

alert(`번개세일! ${randomProduct.name}이(가) 20% 할인 중입니다!`);

return prev.map((p) =>
p.id === randomProduct.id
? {
...p,
price: Math.round(p.price * flashSaleConfig.DISCOUNT_RATE),
}
: p
);
});
}
}, flashSaleConfig.INTERVAL);

return () => clearInterval(interval);
}, flashSaleConfig.INITIAL_DELAY);

// 추천 프로모션
const recConfig = PROMOTION_CONFIG.RECOMMENDATION;
const recTimer = setTimeout(() => {
const interval = setInterval(() => {
if (lastSelectedProduct) {
setProductList((prev) => {
const availableProductList = prev.filter(
(p) => p.id !== lastSelectedProduct && p.stock > 0
);
if (availableProductList.length === 0) return prev;

const randomProduct =
availableProductList[
Math.floor(Math.random() * availableProductList.length)
];

alert(
`${randomProduct.name}은(는) 어떠세요? 지금 구매하시면 5% 추가 할인!`
);

return prev.map((p) =>
p.id === randomProduct.id
? { ...p, price: Math.round(p.price * recConfig.DISCOUNT_RATE) }
: p
);
});
}
}, recConfig.INTERVAL);

return () => clearInterval(interval);
}, recConfig.INITIAL_DELAY);

return () => {
clearTimeout(flashSaleTimer);
clearTimeout(recTimer);
};
}, [lastSelectedProduct]);
Copy link

Choose a reason for hiding this comment

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

useEffect에 의존성배열에 lastSelectedProduct만 들어가있는데, 해당 부분은 프로모션만 영향이 가야 할 것 같은데
번개세일도 영향을 줄 수밖에 없어 보여서 번개 세일과 프로모션을 각각 다른 useEffect에 분리 하는게 어떨까 합니다!
아니면 파일 자체를 분리하는 것은 어떻게 생각 하시나요!

Comment on lines +1 to +30
import { Product, CartItem, CartTotalList } from "../types";
import { PRODUCT_DISCOUNT_LIST } from "../constants";

export const calcTotalList = (
cartItemList: CartItem[],
productList: Product[]
): CartTotalList => {
const initialTotalList: CartTotalList = {
subTotal: 0,
totalAmt: 0,
itemCnt: 0,
discount: 0,
};

return cartItemList.reduce((totalList, item) => {
const product = productList.find((p) => p.id === item.id);
if (!product) return totalList;

const itemSubTotal = product.price * item.qty;
const discount = PRODUCT_DISCOUNT_LIST[item.id] || 0;
const itemDiscount = itemSubTotal * discount;

return {
subTotal: totalList.subTotal + itemSubTotal,
discount: totalList.discount + itemDiscount,
itemCnt: totalList.itemCnt + item.qty,
totalAmt: totalList.totalAmt + (itemSubTotal - itemDiscount),
};
}, initialTotalList);
};
Copy link

Choose a reason for hiding this comment

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

요 친구는 utile에 가깝지 않을까 생각하는데 어떠신가요!

Copy link
Author

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 +32
export const PRODUCT_DISCOUNT_LIST: { [key: string]: number } = {
p1: 0.1,
p2: 0.15,
p3: 0.2,
p4: 0.05,
p5: 0.25,
};

export const BULK_PURCHASE_THRESHOLD = 30;
export const BULK_PURCHASE_DISCOUNT_RATE = 0.25;

export const PROMOTION_CONFIG = {
FLASH_SALE: {
INTERVAL: 30000,
INITIAL_DELAY: 10000,
PROBABILITY: 0.3,
DISCOUNT_RATE: 0.8,
},
RECOMMENDATION: {
INTERVAL: 60000,
INITIAL_DELAY: 20000,
DISCOUNT_RATE: 0.95,
},
};

export const INITIAL_PRODUCT_LIST = [
{ id: "p1", name: "상품1", price: 10000, stock: 50 },
{ id: "p2", name: "상품2", price: 20000, stock: 30 },
{ id: "p3", name: "상품3", price: 30000, stock: 20 },
{ id: "p4", name: "상품4", price: 15000, stock: 0 },
{ id: "p5", name: "상품5", price: 25000, stock: 10 },
];
Copy link

Choose a reason for hiding this comment

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

저는 실무에서 상수 선언하고 as const를 사용하는 방법으로 type도 지정하는데 이 방법 한번 고려해봐주세요!
그리고 해당 변수에 타입이 지정되있지 않으면 생각한것과 다르게 타입추론이 될 수도 있습니다!

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.

잘봤습니다~~ 잘 배우고 갑니다!
이번주차도 고생 많으셨습니다!!

/**
* UI 로직: 이벤트 처리
*/
const EventHandler = {

Choose a reason for hiding this comment

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

오 이렇게 나눌 수도 있구나..! 떠올리지 못했던 방법이라 많이 배우고갑니당👏🏻

Copy link

@hanghaehyunjin hanghaehyunjin Jan 11, 2025

Choose a reason for hiding this comment

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

와웅...!!!!! 저도 생각해보지 못한 방법이라 많이 배우고 갑니당👍🏻👍🏻👍🏻👍🏻👍🏻

availableProducts: [],

initialize() {
this.availableProducts = [
Copy link

Choose a reason for hiding this comment

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

객체를 이렇게 안 써봐서 this로 내부 메서드들 꺼내 쓰는 게 저한텐 낯선데 너무 깔끔하고 좋은 것 같아용..! 배워갑니다 🌟

Comment on lines +143 to +152
const DOMManager = {
elements: {
root: null,
productSelect: null,
addToCartBtn: null,
cartItems: null,
cartTotal: null,
loyaltyPoints: null,
stockStatus: null,
},
Copy link

Choose a reason for hiding this comment

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

UI 어떻게 이렇게 깔끔하게 분리할 수 있죠?!?!? 최고 👍🏻

Comment on lines +425 to 435
if (document.readyState === "loading") {
document.addEventListener("DOMContentLoaded", () => {
DOMManager.initialize();
EventHandler.initialize();
PromotionManager.initialize();
});
} else {
DOMManager.initialize();
EventHandler.initialize();
PromotionManager.initialize();
}
Copy link

Choose a reason for hiding this comment

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

코드가 너무 아름다워요... ✨

@pangkyu
Copy link

pangkyu commented Jan 11, 2025

태영님 극찬듣고 구경왔습니다! 고생하셨어요~


const BULK_PURCHASE_THRESHOLD = 30;
const BULK_PURCHASE_DISCOUNT_RATE = 0.25;
const PROMOTION_CONFIG = {
Copy link

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.

7 participants