FE 코드리뷰 #15
FE 코드리뷰 #15
Conversation
| const [endDate, setEndDate] = useState(null); | ||
| const [focusedInput, setFocusedInput] = useState('startDate'); | ||
|
|
||
| const onDatesChange = ({ startDate, endDate }) => { |
There was a problem hiding this comment.
props의 네이밍 컨벤션과 이벤트 핸들러 네이밍룰은 리액트에서 일반적으로 다릅니다.
전자의 경우에는 on 프리픽스를 달고 후자의 경우엔 handle 프리픽스를 사용합니다. 이러한 컨벤션을 지켜주면 누구나 읽기 좋은 코드가 될거라 생각합니다 :)
| export default ({ personnelItems, requestToServer, personnelControlled, deleteButtonHandle }) => { | ||
| const saveButtonHandler = () => () => { | ||
| const personnel = personnelItems.reduce((payload, item) => { | ||
| const [stateValue, stateName] = [item[0], item[3]]; |
There was a problem hiding this comment.
item[0], item[1]는 personnelItems배열을 파악하고 있지 않으면 어떤 value를 가지는지 파악하기 어렵습니다. 유의미한 변수명을 지정해주는건 어떨까요?
| }; | ||
|
|
||
| const decreaseButtonHandler = (state, setState) => () => { | ||
| if (state === 0) return; |
There was a problem hiding this comment.
state라는 변수명 보다 number 타입을 나타낼 수 있는 변수명을 짓는건 어떨까요?
| return ( | ||
| <Style.PersonnelController> | ||
| {personnelItems.map((state, index) => ( | ||
| <Style.PersonnelItem key={index}> |
There was a problem hiding this comment.
배열의 index로 key 값을 관리하는건 지양해야합니다. 이유는 컴포넌트의 순서가 바뀌게 되면 개발자가 의도한대로 렌더링 되지 않을 수 있기 있습니다. 순서가 바뀌지 않으면 사용할 수도 있겠지만 이는 암묵적으로 개발자가 해당 코드를 외우고 있어야하기 때문에 처음부터 유니크한 아이디로 지정해주면 좋을거 같습니다.
| setState(state + 1); | ||
| }; | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
디펜던시 배열에 필요한 디펜던시를 추가해서 사용해야 개발자가 렌더링을 예측하기 쉽다고 생각하는데 어떻게 생각하세요?
|
|
||
| // Validator | ||
| const isAdultValueLessEqualThan0 = searchOptions => | ||
| searchOptions && searchOptions.personnel && searchOptions.personnel.adult <= 0; |
There was a problem hiding this comment.
옵셔널 체이닝을 이용하면 조금 더 깔끔하겠네요 :)
| searchOptions && searchOptions.personnel && searchOptions.personnel.adult <= 0; | |
| searchOptions?.personnel?.adult <= 0; |
| const nights = date | ||
| ? getNightValueFromDateString({ startDateString: date.checkIn, endDateString: date.checkOut }) | ||
| : 0; | ||
| const guest = personnel ? personnel.adult + personnel.children : 0; |
There was a problem hiding this comment.
useMemo를 통해 값을 메모이제이션 시켜 연산을 줄이는건 어떨까요? 리소스가 많이 드는 연산은 아니지만 작은 변경점이 모여 높은 퍼포먼스를 만든다 생각합니다. :)
| if (error) alert('이미 예약되어있는 숙소입니다'); | ||
| }, [data, loading, error]); | ||
|
|
||
| const nights = date |
There was a problem hiding this comment.
마이너하지만 복수형 변수명을 봤을때 배열 타입이 들어올거 같습니당..
| alert('예약이 완료됬습니다.'); | ||
| dispatchSearchOption({ type: 'reset' }); | ||
| } | ||
| if (loading) console.log('추가 중'); |
There was a problem hiding this comment.
콘솔로그 보다 사용자가 알 수 있는 방법을 이용하는것이 좋아보입니다.
| const decoded = jwt.verify(token, TOKEN_SECRET_KEY); | ||
| return decoded; | ||
| } catch (err) { | ||
| console.log(err); |
There was a problem hiding this comment.
콘솔에 찍는거보다 에러를 throw 해버리는 것도 좋은 방법일 것 같아요! (런타임에서 눈치채는 게 아니라 개발할 때 눈치챌 수 있도록)
No description provided.