-
Notifications
You must be signed in to change notification settings - Fork 13
Feat/week 1 #3
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
base: 강지원
Are you sure you want to change the base?
Feat/week 1 #3
Conversation
sinsehwan
left a comment
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.
추가 과제까지 완전하게 구현해주셨네요! 고생 많으셨습니다. 연관관계 설정까지 적용해주신 것으로 봐서 기본적인 이해도가 있으신 것 같네요 기대하겠습니다.
| private String content; | ||
| private String username; | ||
| private LocalDateTime createdAt; | ||
|
|
||
| @ManyToOne(fetch = FetchType.LAZY) | ||
| @JoinColumn(name="post_id") |
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.
entity에서 DB 스키마 관련 제약조건을 추가해서 더 안전하게 구성할 수 있습니다!
@Column(nullable = false)을 추가해서 의도치 않게 DB에 null값이 들어가는 것을 원천적으로 차단할 수 있습니다.
또한 @ManyToOne(fetch = FetchType.LAZY, optional = false), @JoinColumn(name="post_id", nullable = false, foreignKey = @ForeignKey(name = "fk_comment_post_id_ref_post_id"))형식으로 지정하면 Comment가 항상 연관된 Post를 갖도록 강제하면서 fk 이름까지 명시적으로 지정해 줄 수 있습니다.
// 어노테이션 적용 예시
@ManyToOne(fetch = FetchType.LAZY, optional = false)
@JoinColumn(
name = "user_id",
nullable = false,
foreignKey = @ForeignKey(name = "fk_diary_user_id_ref_user_id")
)
private UserEntity user;| @NoArgsConstructor | ||
| @AllArgsConstructor | ||
| @Builder | ||
| public class Comment{ |
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.
Comment까지 작성해 주셨네요! 좋습니다
|
|
||
| @Entity | ||
| @Getter | ||
| @Setter |
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.
Entity에서 Setter 사용은 지양하는 것이 좋습니다! 왜냐하면, setter를 사용한다는 것은 외부에서 entity 값을 마음대로 수정하도록 하는 것인데, 이는 캡슐화 관점에서 좋지 않습니다. 객체의 값을 직접 수정하지 않고 객체에 수정 작업을 시킨다는 관점으로 코드를 작성하면 더 객체 지향적인 코드가 됩니다! entity에서 setter를 사용하지 않도록 습관 들이면 좋습니다!
| @OneToMany(mappedBy = "post", cascade = CascadeType.ALL,orphanRemoval = true) | ||
| @JsonManagedReference | ||
| private List<Comment> comments = new ArrayList<>(); |
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.
양방향 연관관계를 사용해주셨는데, 양방향 연관관계와 단방향 연관관계의 장단점이 무엇일까요?
| updatedPost.setCreatedAt(existingPost.getCreatedAt()); | ||
| updatedPost.setUpdatedAt(LocalDateTime.now()); |
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.
createdAt은 수정 시 갱신하지 않는 것이 적절해 보입니다! 그리고 현재 @PreUpdate가 적용돼 있어서 수동으로 업데이트하지 않아도 JPA에서 updatedAt을 관리해주기에 updatedAt을 직접 관리하지 않아도 될 것 같습니다!
| @Setter | ||
| @NoArgsConstructor | ||
| @AllArgsConstructor | ||
| @Builder |
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.
Builder 패턴의 경우 호불호가 갈리는 편입니다. Builder를 쓰면 선택적으로 필드를 구성할 수 있다는 장점이 있지만, 컴파일 때 필수 필드에 모두 유효한 값이 들어갔는지 체크하기 어렵다는 단점도 있습니다. 이 점 고려하셔서 사용하시면 될 것 같습니다!
| @Service | ||
| public class CommentService { | ||
| private final CommentRepository commentRepository; | ||
| private final PostRepository postRepository; | ||
|
|
||
| public CommentService(CommentRepository commentRepository, PostRepository postRepository) { | ||
| this.commentRepository = commentRepository; | ||
| this.postRepository = postRepository; | ||
| } | ||
|
|
||
| public List<Comment> getCommentByPost(Long postId){ | ||
| return commentRepository.findByPost_Id(postId); |
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.
현재 @Transactional이 적용되지 않아서 함수 실행 도중 에러가 발생할 경우 안전하게 롤백되지 않을 것 같습니다. 읽기 전용 함수는 @Transactional(readOnly = true), 읽기/쓰기 수행 시 @Transactional 적용해보시면 좋을 것 같습니다!
| public Optional<Post> findById(Long id){ | ||
| return postRepository.findById(id); | ||
| } |
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.
findById에 대한 null 체크를 service 계층에서 미리 수행하고 postService.findById()의 반환 타입을 Post로 수정하면 Controller에서의 Optional 에러 체크 로직을 service 계층으로 옮길 수 있을 것 같습니다! 이렇게 구성하면 controller에서는 db로부터 조회한 객체의 optional 체크를 신경쓰지 않고 단순히 사용자 요청 응답에만 집중할 수 있습니다.
변경점 👍
새로 구현한 기능 및 주요 변경점
게시글에 대해 생성, 조회, 수정, 삭제 기능 구현
댓글 기능을 추가 하였으며 생성, 조회, 수정, 삭제 가능