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

[8팀 조우영] [Chapter 2-2] 디자인 패턴과 함수형 프로그래밍 #14

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

Conversation

yeong30
Copy link

@yeong30 yeong30 commented Jan 14, 2025

과제 체크포인트

기본과제

  • React의 hook 이해하기

  • 함수형 프로그래밍에 대한 이해

  • Component에서 비즈니스 로직을 분리하기

  • 비즈니스 로직에서 특정 엔티티만 다루는 계산을 분리하기

  • Component에서 사용되는 Data가 아닌 로직들은 hook으로 옮겨졌나요?

  • 주어진 hook의 책임에 맞도록 코드가 분리가 되었나요?

  • 계산함수는 순수함수로 작성이 되었나요?

심화과제

  • 뷰데이터와 엔티티데이터의 분리에 대한 이해

  • 엔티티 -> 리파지토리 -> 유즈케이스 -> UI 계층에 대한 이해

  • Component에서 사용되는 Data가 아닌 로직들은 hook으로 옮겨졌나요?

  • 주어진 hook의 책임에 맞도록 코드가 분리가 되었나요?

  • 계산함수는 순수함수로 작성이 되었나요?

  • [] 특정 Entitiy만 다루는 함수는 분리되어 있나요?

  • 특정 Entitiy만 다루는 Component와 UI를 다루는 Component는 분리되어 있나요?

  • 데이터 흐름에 맞는 계층구조를 이루고 의존성이 맞게 작성이 되었나요?

과제 셀프회고

1. 유사한 기능을 가진 함수들의 네이밍 고민

  • 유사하지만 다른 기능을 하는 함수들의 이름을 구분하는것에 어려움을 많이 느꼈습니다. 예를 들어, carts의 합계를 계산하는 다음의 두 함수 calculateCartTotalcalculateCartSum 가 있습니다.
 const calculateCartTotal = (cart: CartItem[], selectedCoupon: Coupon | null) => {
  const totalBeforeDiscount = calculateCartSum(cart, calcTotalPrice);
  const totalAfterDiscount = applyCouponDiscount(calculateCartSum(cart, calculateItemTotal), selectedCoupon);

  const totalDiscount = totalBeforeDiscount - totalAfterDiscount;

  return {
    totalBeforeDiscount,
    totalAfterDiscount,
    totalDiscount
  };
};
export const calculateCartSum = (cart: CartItem[], calculateFn: (item: CartItem) => number) => {
  return cart.reduce((total, item) => total + calculateFn(item), 0);
};

이 두 함수는 모두 장바구니 관련 합계를 계산하지만, calculateCartSum은 개별 아이템의 가격을 더해 총합을 구하는 일반적인 계산 함수이고, calculateCartTotal은 그에 할인 쿠폰을 적용한 최종 합계를 계산합니다. 이 두 함수의 역할이 모두 cart의 total을 계산하는 함수이다 보니 이름을 결정할 때 고민이 많이 되었습니다. 결국 calculateCartSum의 이름에 sum이라는 키워드를 붙여 구분하였지만 사실 이름만으로 두 함수의 역할이 명확히 구분되지 않는다고 느껴집니다. 조금 더 함수 네이밍에 대해 고민해보고 자신만의 컨벤션을 정리해 볼 필요성을 느꼈습니다.

2. 테스트코드 학습

항해 이전까지 테스트 케이스를 작성한 경험이 없기 때문에, 처음 테스트케이스를 통과할 때 어떻게 접근해야할지 막연한 어려움이 있었습니다. 그러나 항해를 통해 테스트케이스를 자주 접하고 이번 과제를 통해 직접 작성해보는 경험을 쌓으며 테스트케이스를 활용하는 방법과 그 장점에 대해 배울 수 있었습니다.
특히 리팩토링을 할 때 테스트의 중요성을 크게 느꼈습니다. 테스트를 돌려가며 진행하니 기존 기능이 잘 유지되고 있는지 바로바로 확인할 수 있었고, 수정으로 인한 버그도 빠르게 찾아낼 수 있었습니다. 덕분에 코드를 수정할 때 다른 사이드이펙트가 없음을 확신하고 리팩토링에 집중할 수 있었습니다.다만 아직까지는 단순한 기능 테스트에만 그치고 있어, 더욱 복잡한 시나리오나 엣지 케이스까지 다룰 수 있는 테스트를 작성하는 것이 목표입니다. 또한, 테스트 커버리지도 높이면서 성능과 안정성까지 검증할 수 있는 테스트를 작성해 보고 싶습니다.

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

  1. 과제를 진행하며 컴포넌트에 사용되는 각 함수들을 hookmodels 하위 중 어디에 위치시키고 정리해야하는지에 어려움을 좀 느꼈습니다. 예를 들어, 장바구니 상태를 기준으로 할인율을 계산하는 getAppliedDiscount 함수는 어디에 위치시키는 것이 적합할지 고민이 많이 되었습니다. 우선 현재는 다음과 같은 기준으로 컴포넌트 훅과 모델의 역할을 다음과 같이 �정리를 해보았는데요,
  • 훅(useCart): state를 업데이트하거나 컴포넌트와 직접 상호작용하는 로직
  • 모델(models/cart): 순수 함수 및 비즈니스 로직

이 기준에 따라 getAppliedDiscount는 state와 직접적으로 연관이 없으므로 models/cart에 두는 방향이 적합하다고 판단했는데, 이러한 기준이 적절한지 확인받고 싶습니다! 혹은, 이 기준을 보완하거나 더 나은 접근 방법이 있다면 조언 부탁드립니다.

  1. 유틸성 함수지만 하나의 Entitiy가 아닌 2개이상의 Entitiy와 관련된 순수함수는 어떻게 접근하면 좋을까요?
    현재 쿠폰의 할인율을 계산하는 applyCouponDiscount 함수는 models/cart의 다른 유틸 함수에서 사용되므로 우선 해당 파일에 함께 두었습니다. 다만, 이 함수는 coupon Entitiy와 관련된 로직이기도 하여 cart 모델에 두는 것이 어색하다고 느껴지기도 합니다.
    이처럼 두 개 이상의 엔터티(cart, coupon)와 관련된 순수 함수는 어디에 위치시키는 것이 좋을까요?
  • 엔터티별로 분리하여 관리
  • 주된 사용 위치 기준으로 관리

두 가지 접근법 중 어떤 기준이 더 적합할지, 또는 다른 효과적인 관리 방안이 있을까요?

Copy link

@osohyun0224 osohyun0224 left a comment

Choose a reason for hiding this comment

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

안녕하세요 우영님 :) 담당 학습메이트 오소현입니다 ~!
항상 우영님의 과제는 깔끔의 정석이십니다 bb 컴포넌트도 역할에 맞게 잘 분리해주셨고, 커스텀 훅으로 분리해볼 수 있는 것도 토글 상태나, 상품 업데이트 하는 로직도 분리해주셔서 넘 좋았습니다 :) 추가 테스트 코드도 잘 작성해주신 것 같습니다! 특히 함수가 각각 단일 책임 원칙 대로 잘 분리해서 구현된 것 같아 너무 좋았습니다 bb

회사 병행하면서 항해 하는 거 쉽지 않은데 ㅠㅠ 이번주 과제도 넘 고생많으셨습니다 우영님!!
다음주 과제도 화이팅입니다 :) 아좌좌 화이팅~!~!

+PS. 그리고 PR에 체크 박스 체크해주세요! 리뷰 받고 싶은 내용도 더 채워주시면 좋을 것 같습니다 :)

Comment on lines +3 to +18
export const useToggleSet = () => {
const [activeIds, setActiveIds] = useState<Set<string>>(new Set());

const toggleItemAccordion = (id: string) => {
setActiveIds(prev => {
const newSet = new Set(prev);
if (newSet.has(id)) {
newSet.delete(id);
} else {
newSet.add(id);
}
return newSet;
});
};

return { toggleItemAccordion, activeIds };

Choose a reason for hiding this comment

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

토글 상태 제어도 커스텀 훅으로 잘 분리해주셨군요 ㅎㅎㅎ

Comment on lines +4 to +22
export const useProductEditForm = () => {
const [formState, setFormState] = useState<Product | null>(null);

const updateFormHandler = (newProductField: Partial<Product>, id: string) => {
setFormState(prev => {
if (prev?.id !== id) return prev;
return { ...prev, ...newProductField };
});
};

const handleEditProduct = (product: Product) => {
setFormState({ ...product });
};

const resetForm = () => {
setFormState(null);
};
return { updateFormHandler, formState, handleEditProduct, resetForm };
};

Choose a reason for hiding this comment

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

상품에 대해서 업데이트하는 것도 커스텀훅으로 분리할 수 있군요 bb

Comment on lines +311 to +316
it('초기 상태는 false', () => {
const { result } = renderHook(() => useAdminState());
expect(result.current.isAdmin).toBe(false);
});

it('토글 함수가 상태를 변경', () => {

Choose a reason for hiding this comment

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

테스트 코드 시나리오를 더 구체적으로 작성해주시면 좋을 것 같아요~! [조건] 하면 [기댓값]이 된다. 예를 들면 요렇게요!

Copy link
Author

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.

2 participants