Open
Conversation
jina4066
commented
Jan 2, 2024
jina4066
commented
k-kbk
reviewed
Jan 3, 2024
|
|
||
| commentService.create(commentCreateRequest); | ||
|
|
||
| return ResponseEntity.ok().build(); |
There was a problem hiding this comment.
post 요청으로 자원이 생성되었으니, ok(200)보다는 created(201)가 더 적합할 것 같네요!
k-kbk
reviewed
Jan 3, 2024
Comment on lines
+16
to
+22
| public Comment toEntity(Member member, Post post) { | ||
| return Comment.builder() | ||
| .member(member) | ||
| .post(post) | ||
| .content(this.content) | ||
| .build(); | ||
| } |
k-kbk
reviewed
Jan 3, 2024
Comment on lines
+17
to
+19
| public static CommentResponse from(Comment comment) { | ||
| return new CommentResponse(comment.getId(), comment.getContent()); | ||
| } |
k-kbk
reviewed
Jan 3, 2024
Comment on lines
+52
to
+54
| return comments.stream() | ||
| .map(CommentResponse::from) | ||
| .collect(Collectors.toList()); |
k-kbk
reviewed
Jan 3, 2024
| public List<CommentResponse> findAll() { | ||
| List<Comment> comments = commentRepository.findAll(); | ||
| if (comments.isEmpty()) { | ||
| throw new NoSuchElementException("댓글이 존재하지 않습니다."); |
There was a problem hiding this comment.
댓글이 존재하지 않을 때, 예외를 던지는 것과 빈 배열은 반환하는 것 중 뭐가 더 적합할지 생각해보면 좋을 것 같습니다!
Member
There was a problem hiding this comment.
게시글 중 댓글이 없는 게시글이 많다는 것을 알면 좋을 것 같아요!
k-kbk
reviewed
Jan 3, 2024
Comment on lines
+32
to
+33
| Member member = memberRepository.findById(commentCreateRequest.getMember()).get(); | ||
| Post post = postRepository.findById(commentCreateRequest.getMember()).get(); |
k-kbk
reviewed
Jan 3, 2024
Comment on lines
+12
to
+14
| private Long member; | ||
| private Long post; | ||
| private String content; |
stophwan
approved these changes
Jan 4, 2024
Comment on lines
+26
to
+28
| private final PostRepository postRepository; | ||
| private final MemberRepository memberRepository; | ||
| private final CommentRepository commentRepository; |
Member
There was a problem hiding this comment.
다른 분 PR에서 이부분 작성해놨으니 참고하시면 좋을 것 같아요!
Comment on lines
+50
to
+53
| public void update(PostUpdateRequest postUpdateRequest) { | ||
| this.title = postUpdateRequest.getTitle(); | ||
| this.content = postUpdateRequest.getContent(); | ||
| } |
Member
There was a problem hiding this comment.
이부분도 다른 분 PR코멘트를 확인해주세요. 변경 가능성이 낮은 도메인 객체가 변경 가능성이 높은 요청객체에 의존하는 것에 대해 고민해보시면 좋을 것 같아요.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.