-
Notifications
You must be signed in to change notification settings - Fork 56
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] 디자인 패턴과 함수형 프로그래밍 #37
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
안녕하세요 도운님~! 담당 학습메이트 오소현입니다 :)
이번주 과제도 엄청 잘해주신 도운님! FSD 대로 관심사 분리도 너무 잘해주시고, 공통 컴포넌트 설계(특히 TextButton!!), 각 함수들의 단일 책임 원칙도 너무 좋았습니다bb 테스트 코드 쪽에도 궁금한거 하나 남겨보았어요!
거의 감탄을 위주로 리뷰를 남겨서 하하 머쓱하네요 아래에 분리하신 내용 한눈에 폴더 구조 확인해보시라고 올려드립니다!
📦src
┃ ┣ 📂components
┃ ┃ ┣ 📜AdminPage.tsx
┃ ┃ ┗ 📜CartPage.tsx
┃ ┣ 📜App.tsx
┃ ┗ 📜main.tsx
┣ 📂refactoring
┃ ┣ 📂app
┃ ┃ ┗ 📂ui
┃ ┃ ┃ ┗ 📜AppContainer.tsx
┃ ┣ 📂entities
┃ ┃ ┣ 📂cart
┃ ┃ ┃ ┣ 📂lib
┃ ┃ ┃ ┃ ┣ 📜cart.ts
┃ ┃ ┃ ┃ ┣ 📜findCartItem.ts
┃ ┃ ┃ ┃ ┗ 📜index.ts
┃ ┃ ┃ ┣ 📂model
┃ ┃ ┃ ┃ ┗ 📜useCart.ts
┃ ┃ ┃ ┗ 📂ui
┃ ┃ ┃ ┃ ┗ 📜CartSummary.tsx
┃ ┃ ┣ 📂coupon
┃ ┃ ┃ ┣ 📂model
┃ ┃ ┃ ┃ ┣ 📜useCoupon.ts
┃ ┃ ┃ ┃ ┗ 📜useCouponContext.ts
┃ ┃ ┃ ┣ 📂provider
┃ ┃ ┃ ┃ ┗ 📜CouponProvider.tsx
┃ ┃ ┃ ┗ 📂ui
┃ ┃ ┃ ┃ ┗ 📜CouponDisplay.tsx
┃ ┃ ┣ 📂discount
┃ ┃ ┃ ┣ 📂lib
┃ ┃ ┃ ┃ ┣ 📜formatDiscountRate.ts
┃ ┃ ┃ ┃ ┣ 📜getMaxDiscount.ts
┃ ┃ ┃ ┃ ┗ 📜index.ts
┃ ┃ ┃ ┗ 📂ui
┃ ┃ ┃ ┃ ┗ 📜DiscountCondition.tsx
┃ ┃ ┗ 📂product
┃ ┃ ┃ ┣ 📂lib
┃ ┃ ┃ ┃ ┣ 📜formatPrice.ts
┃ ┃ ┃ ┃ ┗ 📜index.ts
┃ ┃ ┃ ┣ 📂model
┃ ┃ ┃ ┃ ┣ 📜useProduct.ts
┃ ┃ ┃ ┃ ┗ 📜useProductContext.ts
┃ ┃ ┃ ┣ 📂provider
┃ ┃ ┃ ┃ ┗ 📜ProductProvider.tsx
┃ ┃ ┃ ┗ 📂ui
┃ ┃ ┃ ┃ ┣ 📜PanelTrigger.tsx
┃ ┃ ┃ ┃ ┣ 📜ProductDesc.tsx
┃ ┃ ┃ ┃ ┣ 📜ProductDisplay.tsx
┃ ┃ ┃ ┃ ┣ 📜ProductTitle.tsx
┃ ┃ ┃ ┃ ┗ 📜index.ts
┃ ┣ 📂features
┃ ┃ ┣ 📂coupon
┃ ┃ ┃ ┣ 📂lib
┃ ┃ ┃ ┃ ┗ 📜createCouponOptions.ts
┃ ┃ ┃ ┗ 📂ui
┃ ┃ ┃ ┃ ┣ 📜CouponAddForm.tsx
┃ ┃ ┃ ┃ ┗ 📜CouponApplyBox.tsx
┃ ┃ ┣ 📂discount
┃ ┃ ┃ ┣ 📂model
┃ ┃ ┃ ┃ ┗ 📜useDiscountEditor.ts
┃ ┃ ┃ ┗ 📂ui
┃ ┃ ┃ ┃ ┣ 📂DiscountEditor
┃ ┃ ┃ ┃ ┃ ┗ 📜index.tsx
┃ ┃ ┃ ┃ ┣ 📜BulkDiscountInput.tsx
┃ ┃ ┃ ┃ ┗ 📜EditableDiscountList.tsx
┃ ┃ ┗ 📂product
┃ ┃ ┃ ┣ 📂lib
┃ ┃ ┃ ┃ ┗ 📜getRemainingStock.ts
┃ ┃ ┃ ┣ 📂model
┃ ┃ ┃ ┃ ┗ 📜useProductEditForm.ts
┃ ┃ ┃ ┗ 📂ui
┃ ┃ ┃ ┃ ┣ 📜ProductAddForm.tsx
┃ ┃ ┃ ┃ ┣ 📜ProductEditForm.tsx
┃ ┃ ┃ ┃ ┣ 📜ProductList.tsx
┃ ┃ ┃ ┃ ┗ 📜ProductPanel.tsx
┃ ┣ 📂pages
┃ ┃ ┣ 📂admin
┃ ┃ ┃ ┗ 📂ui
┃ ┃ ┃ ┃ ┗ 📜index.tsx
┃ ┃ ┗ 📂cart
┃ ┃ ┃ ┗ 📂ui
┃ ┃ ┃ ┃ ┗ 📜index.tsx
┃ ┣ 📂shared
┃ ┃ ┣ 📂lib
┃ ┃ ┃ ┗ 📜cn.ts
┃ ┃ ┣ 📂types
┃ ┃ ┃ ┗ 📜SelectOption.ts
┃ ┃ ┗ 📂ui
┃ ┃ ┃ ┣ 📜FormInput.tsx
┃ ┃ ┃ ┣ 📜FormSelect.tsx
┃ ┃ ┃ ┣ 📜TextButton.tsx
┃ ┃ ┃ ┗ 📜index.ts
┃ ┣ 📂widgets
┃ ┃ ┣ 📂coupon
┃ ┃ ┃ ┗ 📂ui
┃ ┃ ┃ ┃ ┣ 📜CouponList.tsx
┃ ┃ ┃ ┃ ┗ 📜CouponManagement.tsx
┃ ┃ ┣ 📂layout
┃ ┃ ┃ ┣ 📜Header.tsx
┃ ┃ ┃ ┗ 📜index.ts
┃ ┃ ┗ 📂product
┃ ┃ ┃ ┗ 📜ProductManagement.tsx
┃ ┣ 📜App.tsx
┃ ┗ 📜main.tsx
이번주 과제하시느라 너무 고생많으셨어요 :) 다음주 과제도 화이팅입니다 ~! 🍀
test('초기 할인 정보가 올바르게 표시되어야 한다 >', () => { | ||
render(<ProductEditForm product={initialProduct} onUpdate={handleUpdate} />); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기에서 테스트 코드가 실행되기 전에 ProductEditForm를 렌더링해주는 부분을 아래의 다른 테스트 에서도 진행하고 있는데, 이 공통적인 부분을 beforeEach 안에 넣어주시면 좋지 않을까 생각이 들었습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
advanced 부분 테스트 코드를 다소 급하게 작성하다보니 아쉬운 부분들이 존재했는데 피드백 감사합니다 !!
const calculateNewCart = (prevCart: CartItem[], product: Product): CartItem[] => { | ||
const existingItem = findExistingItem(prevCart, product); | ||
|
||
if (existingItem) { | ||
return updateCartItemQuantity(prevCart, product.id, existingItem.quantity + 1); | ||
} | ||
|
||
return [...prevCart, { product, quantity: 1 }]; | ||
}; | ||
|
||
const addToCart = (product: Product) => { | ||
setCart((preCart) => calculateNewCart(preCart, product)); | ||
}; | ||
|
||
const removeFromCart = (productId: string) => { | ||
const newCart = cart.filter((cartItem) => cartItem.product.id !== productId); | ||
setCart(newCart); | ||
}; | ||
|
||
const updateQuantity = (productId: string, newQuantity: number) => { | ||
setCart((preCart) => updateCartItemQuantity(preCart, productId, newQuantity)); | ||
}; | ||
|
||
const applyCoupon = (coupon?: Coupon) => { | ||
console.log(coupon); | ||
if (!coupon) return; | ||
setSelectedCoupon(coupon); | ||
}; | ||
|
||
const calculateTotal = () => calculateCartTotal(cart, selectedCoupon); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
분리해주신 함수가 다 단일 책임을 원칙으로 하고 있네요 ㅎㅎ 너무 잘 분리해주신 것 같습니다 bb 나중에 불필요한 콘솔도 지우시면 좋을 것 같아요 !
import { Coupon } from '../../../../types'; | ||
|
||
interface CouponDisplayProps { | ||
coupon: Coupon; | ||
index: number; | ||
} | ||
|
||
export function CouponDisplay({ coupon, index }: CouponDisplayProps) { | ||
const key = `${index}-${coupon.name}`; | ||
|
||
return ( | ||
<div key={key} data-testid={`coupon-${index + 1}`} className="bg-gray-100 p-2 rounded"> | ||
{coupon.name} ({coupon.code}): | ||
{coupon.discountType === 'amount' | ||
? `${coupon.discountValue}원` | ||
: `${coupon.discountValue}%`}{' '} | ||
할인 | ||
</div> | ||
); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
위에서도 CartSummary도 그렇고 각 entities에 맞게 UI 컴포넌트도 잘 분리해서 만들어주신 것 같아요 bb
className={clsx( | ||
'space-y-2', | ||
variant === 'bullet' ? 'list-disc list-inside text-sm text-gray-500' : 'list-none', | ||
)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clsx
를 사용해서 스타일의 조건부 로직을 깔끔하게 구현하고 있네요,, 새로운 지식 알아갑니다 bb 역시 도운님
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clsx를 사용하면 복잡한 스타일링을 좀 더 명확하고 깔끔하게 관리할 수 있어서 좋더라구요
뿐만 아니라 shared/lib 보시면 tailwind merge에 대한 함수도 존재하는데요
export function cn(...inputs: ClassValue[]) {
return twMerge(clsx(inputs));
}
위 twMerge
를 통해 중복되는 스타일링에 대해 오버로드를 자동으로 적용해서 예상치 못한 스타일링 오류를 방지할 수 있어 이것도 좋은 방법인 것 같아요 ㅎㅎ (ex. 'mt-2 mt-4' → 'mt-4' 로 자동 변경)
export interface ProductDisplayProps { | ||
product: Product; | ||
testId?: string; | ||
className?: string; | ||
children: React.ReactNode; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
도운님은 타입 선언해주실 때 선택적 프로퍼티로 처리해야하는 요소랑 그렇지 않은 요소를 꼼꼼하게 고민해서 잘 챙겨주시는 것 같아요 bb 저는 개발하면서 한번 더 생각하면서 선택적 프로퍼티로 처리를 진행하는데 도운님은 항상 고민하시면서 구현해주시는 것 같습니다 bb
<button | ||
onClick={() => updateQuantity(item.product.id, item.quantity - 1)} | ||
className="bg-gray-300 text-gray-800 px-2 py-1 rounded mr-1 hover:bg-gray-400" | ||
> | ||
- | ||
</button> | ||
<button | ||
onClick={() => updateQuantity(item.product.id, item.quantity + 1)} | ||
className="bg-gray-300 text-gray-800 px-2 py-1 rounded mr-1 hover:bg-gray-400" | ||
> | ||
+ | ||
</button> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
요것도 아이콘 버튼으로 분리하면 좋을 것 같긴 한데 만약 분리한다면 폴더 구조의 위치가 궁금하기도 하네요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
맞습니다 ㅠ Admin 부분 리팩토링 이후에 카트 부분은 완전히 마무리하지 못한 것 같아 아쉬움이 있네요
interface ButtonProps extends ButtonHTMLAttributes<HTMLButtonElement> { | ||
variant?: 'primary' | 'add' | 'danger' | 'complete'; | ||
title: string; | ||
fullWidth?: boolean; | ||
isDisabled?: boolean; | ||
className?: string; | ||
testId?: string; | ||
onClick: (event: React.MouseEvent<HTMLButtonElement>) => void; | ||
} | ||
|
||
const buttonStyles = { | ||
primary: 'bg-white text-blue-600 hover:bg-blue-100', | ||
add: 'bg-blue-500 text-white hover:bg-blue-600', | ||
complete: 'bg-green-500 text-white hover:bg-green-600', | ||
danger: 'bg-red-500 text-white hover:bg-red-600', | ||
} as const; | ||
|
||
export function TextButton({ | ||
variant = 'primary', | ||
className = '', | ||
fullWidth = false, | ||
isDisabled = false, | ||
title, | ||
onClick, | ||
testId = '', | ||
}: ButtonProps) { | ||
return ( | ||
<button | ||
data-testid={testId} | ||
onClick={onClick} | ||
className={cn( | ||
'px-4 py-2 rounded', | ||
buttonStyles[variant], | ||
fullWidth && 'w-full', | ||
isDisabled && 'disabled:opacity-50 disabled:cursor-not-allowed', | ||
className, | ||
)} | ||
> | ||
{title} | ||
</button> | ||
); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
버튼 공용컴포넌트 설계 진짜 잘해주셨네요 bb 넘 멋집니다!
테스트 코드 고려도 잘해주시고, 다양한 옵션 fullWidth , isDisabled 고려도 정말 잘해주신 것 같아요 bb
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
도운님! 컴포넌트 분리를 너무 잘 하신 것 같아요!
features
부터 entities
, shared
까지 단계별로 컴포넌트를 적절히 분리시켜서 features
에서는 커스텀 컴포넌트만 사용하는 것 같은데, 그래서 깔끔하게 보이네요!👍
항상 배워갑니다!
import { Coupon } from '../../../../types'; | ||
import { SelectOption } from '../../../shared/types/SelectOption'; | ||
|
||
export const createCouponOptions = (coupons: Coupon[]): SelectOption[] => { | ||
return coupons.map((coupon, index) => ({ | ||
value: index.toString(), | ||
label: `${coupon.name} - ${ | ||
coupon.discountType === 'amount' ? `${coupon.discountValue}원` : `${coupon.discountValue}%` | ||
}`, | ||
})); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오 select
에서 map을 돌려서 options
을 작성하는 게 지저분하다고는 생각했는데, 혹시 같은 이유로 별도의 함수로 빼신 걸까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
네 맞습니다! Option 생성하는 부분을 따로 분리해서 Option에 대한 내용들을 더 명확하게 구분할 수 있게 했습니다
과제 체크포인트
기본과제
React의 hook 이해하기
함수형 프로그래밍에 대한 이해
Component에서 비즈니스 로직을 분리하기
비즈니스 로직에서 특정 엔티티만 다루는 계산을 분리하기
Component에서 사용되는 Data가 아닌 로직들은 hook으로 옮겨졌나요?
주어진 hook의 책임에 맞도록 코드가 분리가 되었나요?
계산함수는 순수함수로 작성이 되었나요?
심화과제
뷰데이터와 엔티티데이터의 분리에 대한 이해
엔티티 -> 리파지토리 -> 유즈케이스 -> UI 계층에 대한 이해
Component에서 사용되는 Data가 아닌 로직들은 hook으로 옮겨졌나요?
주어진 hook의 책임에 맞도록 코드가 분리가 되었나요?
계산함수는 순수함수로 작성이 되었나요?
특정 Entitiy만 다루는 함수는 분리되어 있나요?
특정 Entitiy만 다루는 Component와 UI를 다루는 Component는 분리되어 있나요?
데이터 흐름에 맞는 계층구조를 이루고 의존성이 맞게 작성이 되었나요?
과제 셀프회고
함수형 프로그래밍을 이번 과제를 통해 깊이 있게 학습하기 전에는 "클린 코드"에 대한 이해가 다소 피상적이었습니다. (
React에서 함수형으로 컴포넌트를 개발하는게 함수형 프로그래밍인가..?라는 오해를 하기도 했습니다.) 코드를 작성할 때마다 "클린하게 작성해야 한다"는 나름의 마음가짐이 있었지만 그 필요성에 대한 명확한 근거를 스스로 정립하는 데 상당한 시간이 필요했습니다.클린 코드의 실질적 가치는 여러 측면에서 발견할 수 있었습니다. 코드 가독성 향상을 통한 팀 커뮤니케이션 비용 절감, 유지보수성 개선, 그리고 확장성 확보가 주요했습니다. 특히 계산에 있어 순수 함수를 지향하고 Side Effect를 최소화하는 함수형 프로그래밍의 원칙들이 이러한 가치를 실현하는데 핵심적인 역할을 한다는 것을 깨달았습니다.
하지만 그 전에 몇 가지 기술적 고민이 존재했습니다. 예를 들어 FSD와 같은 아키텍처 패턴은 대규모 어플리케이션에서나 유효한 전략이 아닌지, 계층 분리 기준이나, 컴포넌트의 책임 범위 설정은 어디까지가 적절한지 등 입니다. 따라서 과제에서 FSD를 직접 적용해보며 shared, entities, features, widgets 등의 레이어를 어떤 기준으로 분리하고 각 레이어 간의 의존성을 어떻게 관리할 것인지에 대한 실무적인 고민이 필요했습니다.
이번 과제를 통해 이러한 기법들의 적재적소 사용에 대한 완벽한 해답을 찾지는 못했지만 함수형 프로그래밍의 원칙과 FSD 아키텍처의 실제 적용을 경험하면서 클린 코드의 가치를 더 깊이 이해하게 되었습니다. 특히 계산에 대한 순수 함수 작성, 관심사의 분리 등의 원칙이 코드의 유지보수성을 높이고 실제 심화 과제 테스크 코드 작성에 있어 테스트 용이성을 높이는데 실질적으로 기여한다는 것을 체감했습니다. 또한 단기간에 이러한 클린코드에 대한 노하우를 완벽하게 습득하기란 당연하게도 어려운 것이라는 것을 느꼈습니다. 그래서 앞으로도 각 도메인의 특성과 프로젝트의 요구사항에 맞는 최적의 설계 패턴과 아키텍처를 선택하기 위해 지속적인 학습과 고민이 필요할 것 같습니다.
질문
이번 과제에서 디자인 패턴 또한 주요 주제였으나 디자인 패턴에 관한 부분은 크게 신경쓰지 못한 것 같습니다.. 현재는 FSD 아키텍처를 기반으로 한 폴더 구조 설계와 함수형 프로그래밍 패러다임을 활용한 액션/비즈니스 로직 분리에 초점을 맞추었는데 이러한 접근 방식에서 적용할만한 디자인 패턴이나 유용하게 활용할 수 있는 패턴이 어떤 것들이 있을 지 궁금합니다!