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

[DOTORI] 2주차 두번째 PR #109

Merged
merged 19 commits into from
May 21, 2022

Conversation

mogooee
Copy link
Collaborator

@mogooee mogooee commented May 20, 2022

늦은 마지막 PR 입니다.. 🥲🙏 그동안 리뷰해주셔서 감사합니다 json!

🤔 step4 진행상황

🤹‍♀️ 데모페이지

[ 구현 완료 ]

  1. 자판기 - 금액 투입 기능

    • 자판기에 투입된 금액은 지갑에 맞게 자동 보정된다.
    • 금액을 투입하고 3초간 자판기/지갑에서 추가 투입이 발생하면 타이머를 초기화한다. (디바운싱)
    • 금액을 투입하고 3초간 입력이 없으면 잔돈을 자동 반환한다.
    • 투입된 금액을 알림 문구로 출력된다.
  2. 자판기 - 제품 선택 기능

    • 금액을 투입하고 3초내에 제품을 선택하면 제품을 고를 수 있는 시간(3초)이 초기화된다. (디바운싱)
    • 구매한 제품을 알림 문구로 출력한다.
  3. 자판기 - 잔돈 반환 기능

    • 반환 버튼을 누르면 투입금액이 반환된다.
    • 반환 금액을 알리는 문구가 출력된다.
      -> 현재 디바운싱 코드를 같이 사용하고 있어서 3초 뒤에 출력됨(바로 출력되도록 수정 예정)
    • 거스름돈 로직에 따라 coin을 계산하여 지갑 금액을 추가한다.

[ 구현 예정 ]

  • 잔돈을 지갑으로 반환하는 기능
  • 코드 응집도를 높이는 리팩토링..!

[ 고민하는 점 ]

비동기 기능을 구현하다보니 구조보다는 기능에 중점을 두고 개발을 진행하게 되었습니다. 중복코드도 생기고 코드가 여기저기 분산(?)되어 있는 상태입니다. 주말에는 거스름돈 반환 기능을 마무리하고, 코드 구조를 전체적으로 손보려합니다!

View Compnent에는 최대한 이벤트 핸들러와 return문(구조를 알 수 있는 jsx문법)만 남기는 리팩토링을 진행하려고 합니다. 현재 컴포넌트 내에서 여러개의 context를 가져오는 경우가 있는데, 커스텀훅으로 만들어 가능한 한 하나의 상태, 하나의 로직으로 이용하는 방향으로 리팩토링을 진행해보아도 괜찮까요?

mogooee and others added 19 commits May 10, 2022 15:37
* update: money data

* design: Coin Component

* design: Balance Component

* design: Wallet Component
* update: product data

* design: VendProduct Component

* design: VendController Component

* design: HistoryBox Component

* design: VendingMachine Component

* design: ChangeOutlet Component

* design: InsertCoin Component

* refactor: VendController Component의 하위컴포넌트 분리

* chore: update yarn.lock
* feat: Button Component 구현

* feat: InsertCoin, BrandLabel, Unit에 Button Component 적용

* chore: polished 라이브러리 설치

* refactor: Nav를 NavLink 적용하여 리팩토링
* [DOTORI] 1주차 첫번째 PR (#25)

* chore: 프로젝트 초기 셋팅 (#2)

* feat:VendingMachine, Wallet 라우팅 (#5)

* Wallet Component - Markup (#6)

* update: money data

* design: Coin Component

* design: Balance Component

* design: Wallet Component

* VendingMachine Component - Markup (#9)

* update: product data

* design: VendProduct Component

* design: VendController Component

* design: HistoryBox Component

* design: VendingMachine Component

* design: ChangeOutlet Component

* design: InsertCoin Component

* refactor: VendController Component의 하위컴포넌트 분리

* chore: update yarn.lock

* Common Components 분리(1) (#10)

* feat: Button Component 구현

* feat: InsertCoin, BrandLabel, Unit에 Button Component 적용

* chore: polished 라이브러리 설치

* refactor: Nav를 NavLink 적용하여 리팩토링

* chore: gh-pages 라이브러리 설치

* feat: useDisplay 커스텀 훅 생성

* feat: AppLayout, ToggleDisplay Component 구현

* feat: DisplayProvider 구현

displayMode를 전역 상태로 관리한다.

* design: title font 변경, change outlet title 대문자로 변경
* refactor: components 디렉토리 구조 변경

* feat: common components - Nav 구현

* refactor: 라우터 모듈 분리

* refactor: 사용하지 않는 styled-components 제거, div->span, strong 태그 변경

* refactor: CoinContainer, VendProductContainer Component 분리
* feat: 금액 버튼 클릭시 해당 금액 개수 감소 렌더링

useCoin 커스텀 훅 사용

* feat: 금액 버튼 클릭시 잔고 금액 감소 렌더링

useBalance 커스텀 훅 사용

* feat: CoinProvider 생성

outlet 상위 컴포넌트에 생성하여 라우팅에 따른 상태 초기화 방지

* feat: InsertCoinProvider 구현

지갑에서 선택한 금액을 자판기에 투입된 금액으로 렌더링

* rename: components 디렉토리 구조 수정
coin state로 연산할 수 있는 balance state를 제거
* refactor: setCoin 로직 변경

배열의 카피본에서 같은 값을 갖는 인덱스 요소의 객체를 변경 -> map 사용

* refactor: grid repeat 함수 적용

* refactor: Coin Component useMemo 적용, setProvider 분리

Coin Component 클릭시 금액의 변동사항이 있는 컴포넌트만 리렌더링 된다.
* feat: 지갑에서 금액을 투입하면 알림 문구가 뜬다.

useReducer 적용

* feat: 자판기에 투입된 금액 자동보정 기능 구현

* design: historyBox list margin, 자판기 margin-top

* feat: 자판기에서 금액을 투입하면 알림 문구가 뜬다.

* refactor: 투입 금액 보정 함수 수정

재귀함수로 리팩토링

* feat: 자판기 투입 최소, 최대 금액 설정

0원이면 early return, 잔고보다 큰 금액이면 잔고를 return

* feat: 최대금액 입력시 지갑 잔고 관리

잔고와 동전의 개수는 0으로 초기화된다.

* feat: history provider 분리로 렌더링 최적화

* feat: SelectButton 구현

inputCoin과 stocked에 따라 선택 가능한 상품에 초록불을 렌더한다.

* feat: 선택 가능한 상품을 누를 때 상품명, 잔돈 알림 문구 구현

* feat: 돈을 투입하고 5초간 입력이 없으면 잔돈 자동반환

* feat: 투입금액이 추가 발생하면 타이머 초기화

디바운싱을 이용하여 타이머를 초기화한다.

* rename: setTimer -> setDebounce
* feat: 지갑에서 금액을 투입하면 알림 문구가 뜬다.

useReducer 적용

* feat: 자판기에 투입된 금액 자동보정 기능 구현

* design: historyBox list margin, 자판기 margin-top

* feat: 자판기에서 금액을 투입하면 알림 문구가 뜬다.

* refactor: 투입 금액 보정 함수 수정

재귀함수로 리팩토링

* feat: 자판기 투입 최소, 최대 금액 설정

0원이면 early return, 잔고보다 큰 금액이면 잔고를 return

* feat: 최대금액 입력시 지갑 잔고 관리

잔고와 동전의 개수는 0으로 초기화된다.

* feat: history provider 분리로 렌더링 최적화

* feat: SelectButton 구현

inputCoin과 stocked에 따라 선택 가능한 상품에 초록불을 렌더한다.

* feat: 선택 가능한 상품을 누를 때 상품명, 잔돈 알림 문구 구현

* feat: 돈을 투입하고 5초간 입력이 없으면 잔돈 자동반환

* feat: 투입금액이 추가 발생하면 타이머 초기화

디바운싱을 이용하여 타이머를 초기화한다.

* rename: setTimer -> setDebounce

* remove: vendProductOutlet Component 삭제

* refactor: 투입 금액 보정 로직 변경

잔고가 있는 돈의 배열을 재귀로 돌면서 투입금액에 가까이 누적시킨다.

* feat: 지갑과 자판기에서 금액을 투입하면 선택 시간 3초 초기화

provider에서 useEffect를 사용하여 wallet, vm 컴포넌트에서 insertCoin의 sideEffect를 감지

* refactor: 화살표 함수 컨벤션 통일

* refactor: 중복 코드를 하나의 객체로 변경

* refactor: styled component 분리
@mogooee mogooee added the review-FE New feature or request label May 20, 2022
@mogooee mogooee requested a review from kowoohyuk May 20, 2022 12:01
@mogooee mogooee self-assigned this May 20, 2022
Copy link

@kowoohyuk kowoohyuk left a comment

Choose a reason for hiding this comment

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

도토리 안녕하세요~!
2주간 개발하시느라 고생 많으셨습니다!

리팩토링에 대해서 고민하시는 점이 있으시군요!
모든 구조가 정답은 아니기에 새로운 형태로 수정을 시도하시는 건 정말 좋은 접근법이라 생각합니다!
다만 하나의 상태나 하나의 로직으로 수정했을 때, 결합이 크게 발생하지 않도록 주의하시면 좋을 것 같아요!!

다음 프로젝트를 위해 적어도 휴일 중 하루는 푹 쉬시면서 에너지를 비축하셨으면 좋겠어요 ;)
코드가 깔끔해서 이번에도 자잘한 부분에만 리뷰를 드린 것 같습니다! 👍

Comment on lines +19 to +22
const purchaseProduct = () => {
addHistory("PURCHASE_PRODUCT", { product: name });
setInsertCoin(change);
};

Choose a reason for hiding this comment

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

purchaseProduct가 change만 필요하다면, handleSelectButtonClick 외부로 분리해도 괜찮을 것 같습니다!

Comment on lines +22 to +26
useEffect(() => {
if (!insertCoin) return;
const delaySelectTime = 3000;
setDebounce(autoReturn, delaySelectTime);
}, [insertCoin]);

Choose a reason for hiding this comment

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

디바운스를 useEffect로 주셨군요!
여러 훅들의 적재적소 사용이 돋보여요!! 👍

Comment on lines +30 to +32
let copyCoin = coin;
const totalCoin = coin.reduce((acc, { unit, count }) => acc + unit * count, 0);
let acc = 0;

Choose a reason for hiding this comment

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

지금 형태도 괜찮지만 let대신 recursive에 인자로 전달하는 형태를 고려해도 괜찮을 것 같아요!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

acc 뿐만 아니라 coin 객체도 인자로 전달하는 것이 좋을까요?? 😮

Copy link

@kowoohyuk kowoohyuk May 23, 2022

Choose a reason for hiding this comment

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

가능하다면 둘다 바꿔보시면 좋을 것 같아요! ⭕

@@ -0,0 +1,16 @@
import { useState } from "react";

const useTimer = () => {

Choose a reason for hiding this comment

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

커스텀 훅이 하나 더 추가되었군요!
필요한 부분만 깔끔하게 잘 구현하신 것 같아요 🥇

let abs = 0;
let min = 10000;

if (target > arr[arr.length - 1]) return arr[arr.length - 1];
Copy link

@kowoohyuk kowoohyuk May 21, 2022

Choose a reason for hiding this comment

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

이 부분은 최상단으로 옮기면 어떨까요?! (6번 라인!)


if (target > arr[arr.length - 1]) return arr[arr.length - 1];

for (let i = 0; i < arr.length; i++) {

Choose a reason for hiding this comment

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

for of로 바꿔보면 어떨까요? ✋

@kowoohyuk kowoohyuk merged commit 8021f2a into codesquad-members-2022:dotori May 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review-FE New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants