yeongseoyoon / pomodoro-app Goto Github PK

가볍게 봐주십쇼~

1. 커밋 내역
   왜... 마지막 세 커밋은 gitmoji 안 쓰셨나요? 특별한 이유가 있나요... 뭔가 컨벤션 안 맞아서 조금 불편합니다(?)
2. 폴더 및 컴포넌트 분리
   {Component}.styled.ts 이렇게 분리한 거 꽤나 좋네요. 저도 이렇게 할 걸~ 저도담엔 요렇게 해보겠습니다
3. 커스텀 훅스
   useCounter, useTimer 훅스 따로 만들어서 관리한 부분 꽤나 좋습니다. 과제 요구사항대로 리코일을 안 쓴 부분은 약간 아쉽기는 하지만! 커스텀 훅스를 이용해서 로직 부분을 분리한 부분 너무 좋습니다.
4. 상수값 관리
   constants.ts에 상수값 모두 넣어서 관리하는 부분 좋습니다. 이렇게 하면 시간을 바꾸거나 할 때도 명확하게 어느 부분을 바꿔야 하는지 보이기 때문에 아주 좋은 습관이라고 생각됩니다.

뭐 별로 쓸 게 없네요... 역시 킹영서 갓영서... 졸작도 너무 기대됩니다.

저는 로직의 입장에서 리뷰할께요

1. 이번과제의 로직을 보면 타이머 on/off -> 타이머 -> 시간(초) -> 라운드, 골 이런 흐름을 가지고 있으면 윤영서님 과제의 데이터 흐름도 이와 잘 맞물려서 진행되어 같은 연산을 중복없이 진행하는 효율적인 구조로 짜여져 있습니다. 특히 타이머는 60초만 카운팅하고 그 카운팅된 결과를 분 -> 라운드 -> 골의 흐름으로 잘 전달하고 있어 각 component들이 필요한 만큼의 일을 잘하고 있습니다.

2. 과제에서 제시된 리코일을 쓰지 않아서 구조가 약간 경직된 것으로 보입니다. 물론 이번 과제는 하나의 파일 안에 컴포넌트 들을 다 정리할 수 있어서(ㅎㅎ 저도 그렇게 했습니다. 하지만 제껀 정리도 안되어 있습니다. ㅠㅠ) 그렇게 크게 문제가 되지는 않았는데요. 카운터 훅을 호출해야 하는 곳이 정해져 있어서 만약에 라운드와 골의 depth가 깊어졌다면 prop으로 매우 길게 전달해야 할 것으로 보입니다. 이후에 앱에 새로운 구조가 들어오거나 라운드나 골의 위치를 옮겨야 하면 프로그램 전체에 변경을 가해야 할 수 있습니다.

3. 리코일 같은 전역관리 툴을 이용하여 효율성을 유지하면서 유지 보수성마저 잡는다면 더 좋은 프로그램이 될 것으로 보입니다.

저의 짧은 견해였습니다.

• 링크 드림 -

너무 깔끔하고 명확하게 작성하셔서 놀랐습니다! App.tsx 에 전부 다 때려 넣은 저로써 큰 반성을 하게 만든 코드였습니다..😅
컴포넌트나 로직 분리에 대한 중요성을 잘 몰랐는데 영서님이 분리해 놓은 것을 보니 코드가 더 알아보기 명확하네요.
차후 챌린지 코드 작성 시 저도 조금씩 코드를 분할 해서 작성하는 것을 습관화 해야겠다고 생각했습니다. 많이 배우고 갑니다

저도 useInterval과 useCounter로 커스텀 훅으로 빼고 싶었지만 만들기 급급해서 시도도 못했는데 다 하셨네요. 일단 코드 보면서 더 이해해봐야 할거 같긴한데 코드 잘 보겠습니다.

전체 컴포넌트 리뷰 -> jsx부분에 React.Fragment로 한번 래핑해주셨는데 굳이 할 이유가 없지 않았나 싶습니다. 이미 Wrapper라는 래핑레이어가 있음에도 말이죠.

Home.tsx

이부분은 어떻게 고쳐야할것같긴한데 음... 동일 훅스의 반환값으로 훅스의 인자로 지정되고있어서 의존성이 너무 짙은데 좋은 생각있으면 공유할게요

components/timer/teim.styled.ts
-> Home에서 TimerColonCard를 임포트해와 사용중이신데 TimerConlonCard를 Timer컴포넌트에서 사용하는것도 아닌데 함께 배치하신 이유가 궁금합니다. 저라면 Home.styled.ts에서 만들어서 썼을거같네요

controllButton -> 폴더와 컴포넌트 네이밍이 상이합니다. 이유가 무엇인가요?

Counter -> label 타입을 string해주셨는데 union type으로 타입좁히기를 해주셨으면 어떨까요?
타입추론을 더 강력하게 하는것이 좋아보이네요

useTimer -> timer라는 변수가 useEffect콜백내부로 들어가면 안되는 이유가있었을까용?
TIME이라는 상수의 네이밍이 조금 아쉬운데 MAX_TIME이라덩가 다른건없었을까... 저도 고민되네요

가장위에 const INITIAL_TIME = TIME * 60 as number로 지정해두고 같은 값으로 useState와 resetTime을 해주었으면 어떨까싶네요

<감상평>
오우 잘하셨네요 ㅎㅎ
많이 고민하신 흔적이 보이는 코드라서 자랑스럽네요. 다만 빈 레이어로 wrapping해주는 습관과 컴포넌트와 디렉토리 네이밍에 조금만 더 신경써주시길 바라겠습니다.

1. 사용자 경험이 좋습니다! 부드러운 인터페이스~ 버벅이지 않는 숫자변환
2. ESlint와 vite를 사용해서 프로그래밍하셨는데 저도 언젠가 써보고싶다는 생각..!
3. 컴포넌트를 잘 분리해서 작성해주신 것 좋아용 , useTimer() useCounter()을 따로 빼서 관리하신게 보기 좋았습니다
4. 저는 Minutes를 기본으로 seconds를 만들었는데 seconds => Minutes로 관리하신게 편해보였습니다
5. TimeInterval을 사용하셨군요! 제가 느끼기에는 타임이 빨라진다거나 비정상적이지 않았는데 어떻게 해결하셨는지timeInterval의 기본적인 실제 타임과 잘 맞지않는 부분이 궁금했습니다!
6. 전반적으로 코드가 간결하고 논리적이여서 보기좋았습니다