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-2] 디자인 패턴과 함수형 프로그래밍 #31

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

Conversation

BoYoung00
Copy link

@BoYoung00 BoYoung00 commented Jan 16, 2025

과제 체크포인트

기본과제

  • React의 hook 이해하기

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

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

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

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

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

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

심화과제

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

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

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

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

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

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

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

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

과제 셀프회고

  • 이번 과제는 코드 분리만 계속 했던 것 같습니다. 과제 마감 마지막 날에는 이대로면 안되겠다 싶어서 AI에게 코드 분리 리팩토링을 시키고, 테스트 코드를 처음 작성하는 것이기 때문에 그거에 좀 더 집중해서 작성 했습니다.
    아직 경험이 부족하다 보니 작업 속도가 빠르지는 않지만, 이렇게 차근차근 배우고 쌓아가다 보면 언젠가는 생산성을 높이는 개발자가 될 수 있을 것이라고 믿습니다. 특히, 이번 과제를 통해 클린 코드의 중요성을 다시 한 번 느낄 수 있어 뜻 깊은 시간이었습니다.

과제에서 좋았던 부분

  • 클린 코드의 중요성을 다시 한 번 느꼈습니다.

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

  • Props 드릴링 문제를 해결하기 위해 Compound 패턴을 사용하는 것이 좋다는 이야기를 들었습니다. 직접 이 패턴을 적용해보며 학습할 계획입니다.

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

  • component 폴더 안에서도 각 컴포넌트 파일 간에 부모-자식 관계가 형성되는데, 이를 같은 폴더 안에서 관리하는 것이 좋은 방법인지 아직 명확하게 판단이 서지 않는 것 같습니다..

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

cart: CartItem[],
selectedCoupon: Coupon | null
) => {
const result = useMemo(() => {

Choose a reason for hiding this comment

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

여러가지 역할을 하나의 스코프안에서 동시에 수행하고있는것 같습니다!
역할별로 좀더 나눠서 함수를 만들어사용하는것에 대해 어떻게 생각하시나요?!
예를들어 쿠폰적용하는 부분 과 계산 부분을 나눈다던지 이런방식이요!

Comment on lines +12 to +20
<div className="mb-4">
<label className="block mb-1">{label}: </label>
<input
type={type}
value={value}
onChange={onChange}
className="w-full p-2 border rounded"
/>
</div>
Copy link

Choose a reason for hiding this comment

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

확장성을 고려해서 className도 받을 수 있게 한다면 AddNewCoupon 내부에 있는 필드들도 이 컴포넌트로 충분히 대체가능할 것 같아용!

Copy link

@pangkyu pangkyu left a comment

Choose a reason for hiding this comment

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

이번주 고생많으셨습니다 ! !

@@ -232,13 +234,88 @@ describe('advanced > ', () => {
})

describe('자유롭게 작성해보세요.', () => {
Copy link

Choose a reason for hiding this comment

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

직접 테스트 코드 작성하는 거 좋네요bb

import { Discount, Product } from "../../../../types";

export const useProductToggleButton = (products: Product[], onProductUpdate: (updatedProduct: Product) => void) => {
const [openProductIds, setOpenProductIds] = useState<Set<string>>(new Set());
Copy link

Choose a reason for hiding this comment

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

https://stackoverflow.com/questions/58806883/how-to-use-set-with-reacts-usestate

useState를 사용할 때 Set 객체를 상태로 관리하는거는 권장하지 않는다고 알고있어서 링크하나 첨부드립니다!
-> set이 참조형 데이터구조라서 리액트 상태 변경 감지 매커니즘이랑 호환되지 않아서 사이드 이펙트가 발생할 수 있다고 하네요!
-> 리액트 : 상태변경 감지를 위한 얕은 비교를 수행하는데 Set은 내부 요소 변경이 있어도 참조가 동일하면 변경을 감지하지 못한다

Set대신 배열을 사용해서 상태 관리하고 필요한 경우에 배열 메소드를 사용해서 중복 제거나 추가/삭제하는 것이 리액트 상태관리와 더 잘맞다고 합니다!

Copy link

Choose a reason for hiding this comment

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

이 로직은 순수한 게산 로직처럼 보이는데 hook 대신 유틸함수로 분리하는 것도 좋았을 것 같아요!

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.

5 participants