-
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
♻ add field to MainRecipeResponse #315
Conversation
|
RecipeDataResponse firstResponse = groupedResponses.getFirst(); | ||
boolean mine = firstResponse.authorId() == userInfo.getId(); |
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.
이 부분은 이전에 아토가 얘기한대로 User나 UserInfo에서 확인하도록 해도 좋을 것 같습니다.
(도메인 담당이 다르고 향후 리팩토링할 부분인 것 같아 일단 두었습니다!)
|
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.
좋습니다!
리뷰 하나는 메서드들때문에 너무 무거워져서... 역할에 대해 생각해봐야 할 것 같아서 달았습니다!
카테고리 검색은 옮겨간 끝에 결국 사라졌군요.
좋은 경험이었읍니다,,,
.sorted(Comparator.comparing(MainRecipeResponse::recipeId).reversed()) | ||
.collect(Collectors.toList()); | ||
} | ||
|
||
private MainRecipeResponse getMainRecipeResponse(List<RecipeDataResponse> groupedResponses) { | ||
private MainRecipeResponse getMainRecipeResponse(UserInfo userInfo, List<RecipeDataResponse> groupedResponses) { |
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.
MainRecipeResponse
생성자에서 userInfo
와 groupedResponses
를 파라미터로 받아서 this로 생성해보는 건 어떨까요?
static 메서드가 이제는 필요한 것 같기도 하네요....
다음 오프라인 코드리뷰 때 얘기 한 번 해보시죠!
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로 변경하는 과정이 과하게 느껴지기는 하네요.
팀 컨벤션 정했을 때 static 메서드는 최대한 쓰지 않는 것으로 얘기했던 것 같아 그 부분은 아직 적용하지 않고,
대신 userInfo와 groupedResponses를 가공한 데이터를 파라미터로 하는 MainRecipeResponse 생성자를 추가했습니다.
static메서드를 쓰게 되면 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.
mine
반영 고생하셨습니다!
필드 하나가 추가되었을 뿐인데 역시 이에따른 많은 file change 가 있군요...
연관된 파일들이 침범하지 않고 딱 서비스와 테스트 영역까지 영향이 있으니 적당하다고 생각됩니다!🙌
MainRecipeResponse에 레시피가 조회자(로그인된 사용자)가 작성한 글인지 여부를 나타내는 필드
mine
을 추가했습니다.이 과정에서 현재 미사용중이면서 DTO 변경에 영항받는
readRecipesOfCategory
,readRecipesOfUser
메서드도 삭제했습니다.