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

[8주차] 박수현 #23

Open
wants to merge 19 commits into
base: main
Choose a base branch
from
Open

[8주차] 박수현 #23

wants to merge 19 commits into from

Conversation

Suehyun666
Copy link
Contributor

추가 Q & A

🤔 잘 모르겠거나, 더 궁금한 내용이 있었나요?

집중적으로 리뷰 받고싶은 것이 있나요?

Suehyun666 and others added 16 commits November 4, 2024 11:11
member 클래스입니다.
멤버 컨트롤러입니다.
멤버 서비스입니다.
멤버 레포지토리입니다.
Member 요청과 응답입니다.
Post클래스입니다.
Post컨트롤러입니다.
Post서비스입니다.
Post레포지토리입니다.
Post 요청과 응답입니다.
@Suehyun666 Suehyun666 changed the title [7주차] 박수현 [6,7주차] 박수현 Nov 14, 2024
Post부분입니다.
Member Service입니다.
comment부분입니다!
@Suehyun666 Suehyun666 changed the title [6,7주차] 박수현 [8주차] 박수현 Nov 20, 2024
Copy link
Member

@Hoya324 Hoya324 left a comment

Choose a reason for hiding this comment

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

너무 수고하셨습니다!
PR에 사진을 옮겨주시면 좋을거 같습니다. 방법은 제가 공지해드렸어요!

Comment on lines +21 to +43
private final CommentService commentService;
private final MemberService memberService;
private final PostService postService;
@PostMapping
@ResponseStatus(HttpStatus.CREATED)
public void addComment(@PathVariable String title, @RequestBody CreateCommentRequest createCommentRequest) {
Member member = memberService.findMemberById(createCommentRequest.getUserId());
commentService.addComment(title, member.getId(), createCommentRequest.getContent());}
@GetMapping
public List<CommentResponse> getComments(@PathVariable String title) {
Post post = postService.findPostByTitle(title);
return commentService.getCommentsByPostId(post.getId()).stream()
.map(comment -> new CommentResponse(comment.getId(), comment.getContent(), comment.getMember().getId(), comment.getPost().getId()))
.collect(Collectors.toList());
}
@PutMapping("/{commentId}")
public void updateComment(@PathVariable Long commentId, @RequestBody String content) {
commentService.updateComment(commentId, content);
}
@DeleteMapping("/{commentId}")
public void deleteComment(@PathVariable Long commentId) {
commentService.deleteComment(commentId);
}
Copy link
Member

Choose a reason for hiding this comment

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

각 메서드, 필드 부분마다 개행을 추가해서 가독성을 높여주세료!

Comment on lines +30 to +34
public List<CommentResponse> getComments(@PathVariable String title) {
Post post = postService.findPostByTitle(title);
return commentService.getCommentsByPostId(post.getId()).stream()
.map(comment -> new CommentResponse(comment.getId(), comment.getContent(), comment.getMember().getId(), comment.getPost().getId()))
.collect(Collectors.toList());
Copy link
Member

Choose a reason for hiding this comment

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

commentService에서 하지 않고 controller에서 post를 찾아 보내주는 이유가 있을까요?

만약 post, user, event 등등 찾아야하는 엔티티가 증가한다면 어떻게 리팩토링할까요??

Comment on lines +5 to +8
@Getter
public class UpdateCommentRequest {

}
Copy link
Member

Choose a reason for hiding this comment

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

비어있는 이유가 궁금합니다!

this.userId = userId;
this.postId = postId;
}
}
Copy link
Member

Choose a reason for hiding this comment

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

EOL 지켜주세요!

Comment on lines +25 to +29
Comment comment = Comment.builder()
.content(content)
.member(member)
.post(post)
.build();
Copy link
Member

Choose a reason for hiding this comment

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

정적 팩토리 메서드에 대해 생각해보시면 좋을거 같습니다!


@Builder
private Member(String name, String email, String password) {
this.id = ++idCounter;
Copy link
Member

Choose a reason for hiding this comment

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

해당 도메인과 entity가 함께 존재하는 이유가 있나요??

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants