-
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
Seminar/#2 #6
base: main
Are you sure you want to change the base?
Seminar/#2 #6
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.
수고하셨습니다!!
week02/seminar-main/spring 패키지를 제외한 나머지 파일들은 삭제한 다음에 다시 푸시하고 머지하는게 좋아보여요!
@@ -4,11 +4,12 @@ | |||
import org.sopt.spring.domain.Part; | |||
|
|||
public record MemberFindDto( |
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.
세미나 때 알려준대로 빌더 패턴을 이용해서 만들어도 좋을 거 같아요~!
public List<MemberFindDto> getAllMembers() { | ||
return memberRepository.findAll().stream() | ||
.map(MemberFindDto::of) | ||
.collect(Collectors.toList()); |
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.
현욱님 혜린님한테 남긴 리뷰 똑같이 남깁니다!
.collect(Collectors.toList()); 보다는 .toList(); 를 사용하는 것이 더 좋아보입니다!
자세한 내용은 아래 링크로 첨부하나 결론적으로 .toList()를 사용하는 것이 더 좋아보이는 이유는 .collect(Collectors.toList())는 불변성을 보장하지 못하는 반면 .toList()는 불변성을 보장하기 때문입니다.
@@ -36,4 +39,11 @@ public void deleteMemberById(Long memberId) { | |||
.orElseThrow(() -> new EntityNotFoundException("ID에 해당하는 사용자가 존재하지 않습니다.")); | |||
memberRepository.delete(member); | |||
} | |||
|
|||
public List<MemberFindDto> getAllMembers() { |
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.
혜린님한테 남긴 리뷰 똑같이 남깁니다!
List형을 반환하는 것보다는 List를 가지고 있는 또 하나의 새로운 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.
깃도 어려우셨을텐데 과제하느라 고생하셨습니다!
다른 분들의 PR을 살펴보면 201,204의 경우에도 Response 객체를 보내는 예시가 많으니 참고해보면 좋을 것 같네요 :)
저도 세미나 자료를 준비하면서 안 부분인데 네이버의 경우는 status나 다른 내용들을 body에 노출하지는 않더라고요! 회사 내규/협업 환경마다 다른 것 같아요! 여러 방식을 공부해보는걸 추천드립니다!
@@ -43,4 +44,9 @@ public ResponseEntity deleteMemberById(@PathVariable Long memberId){ | |||
return ResponseEntity.noContent().build(); | |||
} | |||
|
|||
@GetMapping |
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.
혁진님께도 동일한 코멘트 남겼었는데 전체 멤버 리스트를 반환한다는 것을 보이기 위해 url 추가해보시는 건 어떨까요?
이 주의 과제
모든 멤버를 List 형태로 반환하는 GET API 구현 및 API 명세서 작성
MemberController.java
@GetMapping 어노테이션을 사용하여 HTTP GET 요청을 처리하는 메서드입니다. 이 메서드는 /members 로 들어오는 GET 요청을 처리하고, 모든 회원 정보를 리스트 형식으로 반환합니다.
MemberService.java
MemberRepository에서 모든 회원을 가져와서 MemberFindDto로 매핑한 후, 해당 DTO의 리스트를 반환하는 getAllMembers() 메서드입니다.
결과 창입니다.
localhost:8080/api/v1/member
요구사항 분석
POST - 멤버 가입 api
Request
Request Header
Request Body
Response
Response Body
GET - 단일 멤버 조회 api
Request
Path Parameter
Response
Response Body
DELETE- 멤버 삭제 api
Request
Path Parameter
Response
Response Body
GET - 모든 멤버 조회 api
Request
Response
Response Body
구현 고민 사항
제가 따로 프로젝트를 진행했을 때는 HTTP status code를 반환해서 출력해주는 형식으로 코드를 작성했었습니다. (예: 200, 201, 400 등을 반환) 하지만 세미나에서 진행한 코드를 포스트맨을 사용할 경우
위에 status 창이 뜨긴 하지만 따로 201을 출력해주지는 않습니다. 저는 HTTP status code를 출력해주는 형식으로만 코드를 작성해봐서 반환되는 값이 명시적으로 보이지 않으면 뭔가 어색하더라고요... 그래서 다른 분들은 개발하시면서 이러한 형식을 출력하는 것에 대해 어떻게 생각 하시는지 많은 분들의 의견을 들어보고 싶습니다.
질문있어요!
또한 intellij에서 commit 올릴 때 change 된 사항들 전부를 올리는 편인데 그냥 코드만 올려도 되는지 궁금합니다. 전부를 올리지 않았을 때 부작용 같은것도 궁금합니다...!
감사합니다.