Skip to content
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

Feat #75 멤버 리스트 반환 수정 및 멤버 상세 조회 추가 #75

Conversation

huncozyboy
Copy link
Member

@huncozyboy huncozyboy commented Nov 20, 2024

PR 내용

멤버 리스트 조회 API
멤버 상세 조회 API

PR 세부사항

기존 API를 수정하여 멤버 리스트를 반환할 때 데이터 양을 축소하였습니다
SummaryResponse를 통해 userId, name, cardinals, department 만 반환하도록 수정했습니다
GET으로 축소된 멤버 리스트 반환하는 내용 구현하였고,
멤버 상세 조회 API 내용도 구현하였습니다
기존 dto에 있는 Response 내용을 참조하기 보다는
새로운 UserResponse를 생성하여 멤버 상세 조회 API시 반환하도록 하였습니다

관련 스크린샷

image

주의사항

로직은 이전에 adminResponse 축소할때와 동일하게 진행 하였으나,
UserAdminController 반환 리스트가 아닌 UserController의 동아리 멤버 전체조회(전체/기수별) api를 올바르게 수정했습니다.
기존 findAll메서드와 AdminResponse는 그대로 유지하기 위해서 새로운 findAllUser 메서드를 추가하여 구현했습니다


체크 리스트

  • 리뷰어 설정
  • Assignee 설정
  • Label 설정
  • 제목 양식 맞췄나요? (ex. #0 Feat: 기능 추가)
  • 변경 사항에 대한 테스트

@huncozyboy huncozyboy self-assigned this Nov 20, 2024
@huncozyboy huncozyboy changed the title Feat/#72/멤버 리스트 반환 수정 및 멤버 상세 조회 추가 Feat #72 멤버 리스트 반환 수정 및 멤버 상세 조회 추가 Nov 20, 2024
@huncozyboy huncozyboy changed the title Feat #72 멤버 리스트 반환 수정 및 멤버 상세 조회 추가 Feat #75 멤버 리스트 반환 수정 및 멤버 상세 조회 추가 Nov 20, 2024
Copy link
Member

@hyxklee hyxklee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생하셨어요!
제 생각을 적어봤으니 읽어봐주시면 감사하겠습니다
그리고 스웨거에 실제 데이터를 넣어서 멤버 목록 조회/ 상세 조회해서 실제로 반환되는 값을 스크린샷으로 부탁드릴게요!

public CommonResponse<Response> findUser(@RequestParam Long userId) {
return CommonResponse.createSuccess(
USER_DETAILS_SUCCESS.getMessage(),
userUseCase.findUserDetails(userId)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

의도된 줄바꿈일까요? 길이가 길지 않아서 기존 코드 형식과 통일해도 괜찮을 것 같아요!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

미처 확인하지 못했던 부분입니다
가독성 위해서 다른 컨트롤러 형식들과 통일하겠습니다

@Override
public Response findUserDetails(Long userId) {
User user = userGetService.find(userId);
if (user == null) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

userGetService.find() 메서드 내부에서 null 체킹을 해서 예외를 던져주고 있기 때문에 useCase 단에서 따로 체크는 하지 않아도 될 것 같아요!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

넵 해당 로직 확인하여 수정했습니다

Copy link
Collaborator

@jj0526 jj0526 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수고하셨습니다!

.flatMap(user -> Stream.concat(
user.getCardinals().stream()
.map(cardinal -> new AbstractMap.SimpleEntry<>(cardinal, mapper.toSummaryResponse(user))), // 기수별 Map
Stream.of(new AbstractMap.SimpleEntry<>(0, mapper.toSummaryResponse(user))) // 모든 기수는 cardinal 0에 저장
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mapper.toSummaryResponse(user)을 반복적으로 사용하게 되는데 이부분을 한번만 호출하는건 어떨까요?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

해당 로직은 기존에 구현되어있던 로직과 동일한 형태로 작성했는데, 수정하지 않은 이유는 mapper.toSummaryResponse가 동일한 user 객체를 받아온다는 가정이므로 여러번 호출하더라도 성능저하가 되지 않는다고 판단했었고, 각 스트림 역할을 명확하게 읽을 수 있어 가독성이 더 좋을거같다는 생각을 했는데 어떻게 생각하시나요 ??

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

구글링 해보니 동일한 객체를 반환하는 메서드를 반복적으로 호출하는 것만으로도 오버헤드가 발생되네요
해당 내용도 수정하겠습니다

Copy link
Member

@hyxklee hyxklee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생하셨어요!

@huncozyboy huncozyboy merged commit 327a913 into develop Nov 23, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feat #72 기존 멤버 리스트 반환 수정 및 멤버 상세 조회 API 추가
3 participants