Conversation
sinsehwan
left a comment
There was a problem hiding this comment.
고생하셨습니다! 여러 요소 고민해보고 잘 작성해주신 것 같네요! 개선할 부분 몇 개 남겼습니다~
| public class CommentRequest { | ||
| private String content; | ||
| private String username; | ||
|
|
There was a problem hiding this comment.
DTO는 class 대신 Record를 사용해서 작성하는 걸 추천드립니다! 불변 객체라 계층 간 값 전달 시 안전하게 사용 가능합니다.
| public CommentResponse create(Long taskId, CommentRequest req) { | ||
| Task task = taskRepo.findById(taskId) | ||
| .orElseThrow(() -> new IllegalArgumentException("Task not found")); | ||
| Comment comment = new Comment(req.getContent(), req.getUsername(), task); | ||
| Comment saved = commentRepo.save(comment); | ||
| return new CommentResponse(saved); | ||
| } |
There was a problem hiding this comment.
안정적인 비즈니스 로직 수행을 보장하기 위해서 서비스 계층에서 @transaction을 적용하는 게 좋을 것 같습니다! readOnly 옵션에 대해서도 추가적으로 공부해보면 좋을 것 같아요
| private String content; | ||
| private String username; |
There was a problem hiding this comment.
만약 content가 1000자 이상이라면 DB에서 에러가 날 것 같습니다! 입력 값에 대해서는 항상 validation을 적용하는 게 좋습니다. @Valid와 validation관련 어노테이션을 공부해보시면 좋을 것 같네요
| @OneToMany(mappedBy = "task", cascade = CascadeType.ALL, orphanRemoval = true) | ||
| private List<Comment> comments = new ArrayList<>(); |
There was a problem hiding this comment.
양방향 연관관계 설정 시 무한루프에 빠지기 쉽습니다. 저도 예전에 그랬던 적이 있기도 하고 사실 단방향 연관관계만(ManyToOne만 적용) 설정하더라도 DB 테이블 구조는 같게 구성됩니다. 그래서 꼭 필요한 게 아니라면 단방향으로 구성하는 게 관리하기 쉽습니다.
| if (r.title != null) t.setTitle(r.title); | ||
| t.setDescription(r.description); | ||
| t.setDueDate(r.dueDate); | ||
| t.setPriority(r.priority); | ||
| t.setStatus(r.status == null ? Task.Status.TODO : r.status); |
There was a problem hiding this comment.
DTO를 Record로 만들고 dto에 entity 변환 로직을 구성하거나 mapper 등을 두면 서비스 로직을 간결하게 만들 수 있습니다.
| public Task update(Long id, TaskRequest r) { //부분 수정 가능 | ||
| Task t = findById(id); | ||
| if (r.title != null) t.setTitle(r.title); | ||
| if (r.description != null) t.setDescription(r.description); | ||
| if (r.dueDate != null) t.setDueDate(r.dueDate); | ||
| if (r.priority != null) t.setPriority(r.priority); | ||
| if (r.status != null) t.setStatus(r.status); | ||
| return t; // JPA dirty checking | ||
| } |
There was a problem hiding this comment.
해당 부분은 task 클래스에서 처리하는 게 더 좋을 것 같습니다! setter를 사용하기보단 findById로 task를 찾고 task.update를 호출해서 task 내부적으로 처리하면 더 깔끔해질 것 같아요
| @PathVariable Long taskId, | ||
| @PathVariable Long commentId) { | ||
| service.delete(commentId); | ||
| return ResponseEntity.noContent().build(); |
변경점 👍
게시글 CRUD 구현
버그 해결 💊
처음에 comment 안에 task가 있고 task 안에도 List가 있도록 코드를 짜서 comment를 달면 무한루프에 빠짐.
ㄴ Dto 안에 response를 따로 만들어서 해결.
테스트 💻
아직 spring boot의 전체적인 흐름에 대해서 헷갈리는 부분이 많아 기초부터 차근차근 열심히 공부해보겠습니다.