과제 1 - Counter 앱 만들고 파일 분리하기 #115
Conversation
moonkii
left a comment
moonkii
left a comment
There was a problem hiding this comment.
PR을 보내시기 전에 린트를 항상 확인해주세요.
PR을 보내고나서는 Files changed 를 통해 항상 셀프리뷰를 해보시면 좋아요.
점진적으로 개발해 나가는건 좋은 것 같아요.
필요에 따라 하나씩 분리해나가면 될 것 같아요 ;)
src/index.jsx
Outdated
| @@ -1,0 +1,54 @@ | |||
| import React, { useState } from "react"; | |||
There was a problem hiding this comment.
자바스크립트에서는 주로 single quote 를 사용해요.
src/index.jsx
Outdated
| } | ||
|
|
||
| function Buttons({ onClick }) { | ||
| const nums = [1, 2, 3, 4, 5]; |
There was a problem hiding this comment.
저는 함수나 변수명을 지을 때 축약형을 사용하는 걸 지양해요.
코드를 읽는 사람으로 하여금 인지자원을 더 소모하게 하고, 코드의 가독성을 떨어뜨리기 때문이에요.
There was a problem hiding this comment.
리뷰 감사합니다!
numbers로 풀네임으로 변경했습니다!
src/index.jsx
Outdated
| function App() { | ||
| const [state, setState] = useState({ | ||
| count: 0 | ||
| }) | ||
| const { count } = state | ||
|
|
||
| function handleClick(num) { | ||
| setState({ | ||
| count: count + num | ||
| }) | ||
| } | ||
|
|
||
| return ( | ||
| <Page onClick={handleClick} count={count} /> | ||
| ) | ||
| } |
There was a problem hiding this comment.
컴포넌트들을 만들어보면서 필요할 때 마다 분리해 나가는 점진적인 개발을 하는 것도 좋아요! 👍
각각 컴포넌트들을 파일로 구분해서 분리해주시면 될 것 같아요.
리뷰eslinteslint가 자동으로 되는줄 알았습니다.. 앞으로 잘 체크하겠습니다. 셀프리뷰"PR을 보내고나서는 Files changed 를 통해 항상 셀프리뷰를 해보시면 좋아요." 늘 감사합니다! 커밋 내용chore로 도배를 했는데 다음엔 refactor 로 입력하겠습니다 |
src/components/App.jsx
Outdated
| import React from 'react' | ||
| import { useState } from 'react'; | ||
| import Page from './Page.jsx' |
There was a problem hiding this comment.
| import React from 'react' | |
| import { useState } from 'react'; | |
| import Page from './Page.jsx' | |
| import React from 'react' | |
| import { useState } from 'react'; | |
| import Page from './Page.jsx' |
의미상으로 구분되는 경우 빈 줄을 넣어 구분해주시면 좋아요.
src/components/App.jsx
Outdated
| setState({ | ||
| count: count + num | ||
| }) |
There was a problem hiding this comment.
만약 객체에 다른 속성이 있으면 어떤 문제가 발생할 수 있을까요?
상태를 객체나 배열로 관리해주실 때는 업데이트 시 기존의 상태를 포함시켜줘야해요.
src/components/NumberButton.jsx
Outdated
| function NumberButton({ onClick, children }) { | ||
| return ( | ||
| <button onClick={() => onClick(children)} type='button'>{children}</button> | ||
| ) | ||
| } |
There was a problem hiding this comment.
버튼 컴포넌트를 재사용할 수 있는 구조로 변경하여 MainClick 컴포넌트까지 대체해볼 수 있을까요?
src/components/Buttons.jsx
Outdated
| return ( | ||
| <div> | ||
| {numbers.map((i) => ( | ||
| <NumberButton key={i} number={i} onClick={onClick}>{i}</NumberButton> |
There was a problem hiding this comment.
| <NumberButton key={i} number={i} onClick={onClick}>{i}</NumberButton> | |
| <NumberButton | |
| key={i} | |
| number={i} | |
| onClick={onClick}> | |
| {i} | |
| </NumberButton> |
저는 속성이 2개 이상일 때는 줄바꿈해주면 코드가독성이 더 좋다고 생각하고 코드를 정리해주는 편이에요.
코드 가독성을 고려한 자기만의 코드 정리 기준을 세우시면 좋아요.
src/components/NumberButton.jsx
Outdated
|
|
||
| export default function NumberButton({ onClick, number, children }) { | ||
| return ( | ||
| <button onClick={() => onClick(number)} type="button">{children}</button> |
There was a problem hiding this comment.
number를 클릭 이벤트 핸들러 함수의 매개변수에 넣어주는건 상위 컴포넌트에서 처리해주게 되면 여기서는 좀 더 재사용이 가능한 버튼 컴포넌트를 만들 수 있을 것 같네요.
onClick props에 매개변수를 직접 넣어서 구현했다.
num을 numbers로 바꾸었다.
moonkii
left a comment
moonkii
left a comment
There was a problem hiding this comment.
코드 가독성도 더 좋아지고 코드도 더 정돈된 것 같아요 👍
회고작성 후 공유해주시면 merge 하도록하겠습니다!
리뷰감사합니다. |
2 participants
리뷰
1주차 과제에서는 콜백 함수에 변수를 넣지 않고 구현했지만
데이터를 최상단 컴포넌트인 App에서 관리해야함으로
하위 컴포넌트들이 최상단의 컴포넌트의 데이터를 공유한다는 생각으로
이번에는 넣어서 구현해보았습니다.
고칠점
너무 한번에 커밋한 것같아 다음에는 좀더 세분화해서 커밋하도록 하겠습니다.
기능