solightnsalt / todolist Goto Github PK

먼저 다솔님의 코드 잘 읽었습니다! 기능 컴포넌트의 네이밍도 적절하고 새 todo의 id를 uuidv4로 지정하는 등의 코드 구현이 제가 구현한 코드와는 다르게 새로운 관점으로 해석할 수 있어서 좋았습니다. 추가적으로 수정한다면 조금 더 좋은 코드가 될 수 있을 것 같아서 말씀드립니다 😊

1. App.css 안의 style들이나 컴포넌트 내의 하단에 구현하신 style들은
   별도의 style 파일에 분리하시면 수정에 더 용이하실 것 같습니다.
2. ListWrapper.jsx 파일의 return 부분은 working 부분과 done 부분의 두 StList은
   삼항연산자 등의 방법을 이용하면 코드를 조금 더 줄일 수 있을 것 같습니다.

도움이 되셨길 바랍니다! 💖

redux로 전역상태 관리를 하셨지만,
AddForm.jsx 에서는 useState로 2중으로 상태관리를 하시는것 같습니다!

컴포넌트 분리도 잘하셨지만, 기회가 있다면 스타일도 같이 분리해서 작업해보시는것도 좋을것 같아요!
분리해서 작업하면, CSS와 기능 구현이 분리되어 가독성이 좋아지는것 같아서 권해보았습니다!

Redux의 Ducks 구조를 잘 적용하시고, 부가적인 설명이 있으신게 코드 이해하기 좋은것 같아요!

1. AddForm.jsx 에서 onSubmitHandler 가 동작 할 때 dispatch 해주고 난 뒤 setTodo로 초기화 시켜주는 작업을 하는데요. 여기서 리셋 해주는 부분을 또 따로 함수로 빼서 onSubmitHandler에서 마지막에 불러주면 한눈에 보고 코드 파악할 수 있어서 가독성이 더 좋을 것 같아요!

2. ListWrapper.jsx 에서 todolist.filter((todo) => !todo.isDone) 같은 부분을 return 위에서 따로 변수로 빼주면 좋을 것 같아요! 
ex) doneTodoList = todolist.filter((todo) => !todo.isDone);
이렇게 빼주면 가독성도 좋아지고 변수명에서 뜻을 알 수 있으니 굳이 주석으로 진행중, 완료된 이러한 설명이 필요 없겠죠?

3. Detail 페이지에서는 dispatch 로직이 있고, Home 페이지에서는 로직이 아예 없게 코드를 짜셨는데 통일성을 가져가면 좋을 것 같아요!
Detail 페이지에서 StTodo부터를 DetailContent(예시)와 같은 컴포넌트로 빼주고 그 컴포넌트에서 필요한 데이터를 불러오면 Detail 페이지에서는 컴포넌트 분리가 확실히 돼서 가독성도 좋고 Home 페이지와도 통일성을 가질 수 있어서 더 좋을 것 같네요!

이번 과제도 고생하셨습니다~

• Form, Header, List로 각각의 컴포넌트 분리가 잘되어 있어 재상용성이 높은점과 각각의 기능을 찾아서 보기에 편리했습니다.
• 늘 함수를 작성한 이후에, export default <컴포넌트명>으로 컴포넌트를 매핑해주는 방법만 알고 있었는데 다른 방법이 있다는 것을 배울 수 있었습니다.
• id 생성시 Date.now()를 사용하거나, length를 사용하는 방법만 생각했었는데 uuid를 사용해 난수 생성을 한 부분을 보고 감탄했습니다.
• 삭제, 상태변경에만 신경 썼었는데 console.log로 각각의 상태를 출력해주면서 로그처리를 해놓은 부분이 개발자로서 배워야 겠다고 생각했습니다.
• 클래스명이나 함수명이 명확하게 지정되어 있어, 어떤 의도를 나타내는지 알기에 편리했습니다.
• App.jsx에서 기능별 주석이 있다면 더 보기 편리할 것 같습니다.

1. titleInputHandler 등의 함수에서 쓰이는 event.target.value 와 같은 친구들은 변수로 따로 빼주고 set 함수에 넣어주면 좋을 것 같아요. 각 함수 이름에서도 유추 할 수 있겠지만 나중에 더 복잡한 로직을 짜게되면 이렇게 이름이 긴 친구들은 짧은 변수명으로 명시해주면 가독성이 더 좋아집니다. 
Ex) const title = event.target.value;
2. 컴포넌트 분리를 엄청 잘 해주셨는데, Form 컴포넌트 안에 있는 label, input 들을 하나의 세트로 컴포넌트 분리를 더 해주면 좋을 것 같습니다! 그만큼 반복되는 코드를 줄일 수 있을 것 같아요!
3. ListWrapper 컴포넌트에서 isDone 인 경우 빈 fragment 를 반환하고 있는데 왜 저렇게 사용하셨을까요? 이유가 있나요? 만약 아무것도 보여지지 않게 하는게 목적이라면 null을 return 해도 됩니다!
4. 코드를 쭉 읽어보니까 3항 연산자로 done이 true인 경우 false인 경우를 따로 구분하기 위해서 이렇게 하신 것 같은데 다른 방법도 알려드릴게요!
   현재 코드 : item.IsDone ? ( 코드들 ) : <></>
   다른 방법의 코드 : item.isDone && ( 코드들 )

첫 리액트 과제 하시느라 고생 많으셨습니다!