-
Notifications
You must be signed in to change notification settings - Fork 0
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
DTO 디렉토리 구조 변경 #166
DTO 디렉토리 구조 변경 #166
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
폴더로 나눈건 정말 좋은 것 같습니다.
추가적으로 고려해보면 좋을 것 같은게 있는데,
- 파일 이름에 DTO가 붙은것과 안붙은것의 차이는 무엇인가? (DTO는 계층간 데이터 이동용? 계층 이동할때만 쓰이는가?)
- 한 파일에 여러 클래스가 들어있는것과 하나만 들어있는것이 섞여있는데 이유가 있는가?
이 부분에 대해서 이야기를 나눠보면 좋을 것 같습니다.
각 질문에 대해 제 대답을 적자면,
|
지금
말씀대로라면 이들 두 그룹 중 하나는 이름 변경이 필요해 보입니다.
저는 일단 다른 파일들은 다 분리가 되었으니 마찬가지로 분리가 되어야한다고 생각합니다. 하지만 UI에서 쓰려고 만든 클래스긴 하지만 결과적으로 Data Layer에서 전달해주는 데이터 타입이기 때문에 ui 패키지 하위로 옮기는게 옳은지는 모르겠습니다. |
그렇다면 파일 분리는 하는 것이 좋아보이고, ui.data가 별로라면 data.ui에 두는 것은 어떠신가요? |
Repository도 data영역으로 포함되니 ProductVerifyDTO는 확실히 변경이 필요해 보이네요. data.ui폴더 보다는 이전에 말씀하신 ui.data가 더 의미적으로 맞는 것 같습니다. 죄송합니다 😅 |
넵, 알겠습니다. 오늘 중으로 추가 커밋 보낸 후 다시 리뷰 요청 드리겠습니다! |
GraphDataConvertor는 data 패키지와 Graph 패키지 간 데이터 전환을 담당하므로 두 패키지와 같은 레벨에 있도록 변경
errorState는 dto가 아니라 network 관련 데이터이므로 해당 패키지로 이동
…iceGuard into refactor/and/dto
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수고하셨습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
확실히 개선이 많이 된 것 같네요😄 어려운 부분들 혼자서 리팩토링하시느라 고생하셨습니다!
진행 내용
스크린샷 (선택)