refactor: 투두 api, 로직 수정 및 테스트 수정 #241
Conversation
|
스터디 시작전에 투두 추가 못하게해야하는데 그러려면 스터디 도메인을 불러와야할까요.. |
| return todo.getId(); | ||
| public void createNecessaryTodo(Member member, Long roundId, TodoCreateRequest request) { | ||
| Round round = findRoundById(roundId); | ||
| round.createNecessaryTodo(member, request.content()); | ||
| } | ||
|
|
||
| private Round findRoundById(Long roundId) { |
There was a problem hiding this comment.
p3: findRoundById() 메서드의 위치가 아래로 이동되어야 할 것 같아요~
| OptionalTodo todo = findOptionalTodoById(todoId); | ||
| round.findRoundOfMemberBy(member) | ||
| .validateTodoOwner(todo); |
There was a problem hiding this comment.
p3: 현재 updateOptionalTodo() 메서드에서는
- (Repository) 라운드 조회
- (Repository) 투두 조회
- (Round) ROM 조회
- (ROM) 투두 작성자 검증
이러한 과정을 거치고 있는 것 같습니다.
여기서 OptionalTodo 조회를 Repository가 아닌, ROM으로 하면 어떨까요?
그 이유는 첫 번째로, ROM이 OptionalTodo를 참조하고 있기 때문에 굳이 한 번 더 DB를 조회하지 않아도 될 것 같습니다.
두 번째로, 현재 RoundOfMember.validateTodoOwner() 에서는 contains()를 통해 투두 작성자 검증을 하고 있습니다.
만약 위에서 제안했듯이, 투두를 Repository가 아니라 ROM에서 꺼낸다면 기존의 투두 작성자 검증까지 한 번에 할 수 있을 것 같습니다.
즉, ROM에서 OptionalTodo를 꺼낸다면 updateOptionalTodo() 메서드는 다음과 같은 흐름이 될 것 같아요.
- (Repository) 라운드 조회
- (Round) ROM 조회
- (ROM) OptionalTodo 조회 -> 투두 작성자는 자연스럽게 검증 가능
There was a problem hiding this comment.
엇 이거 방금 push된 커밋 확인했는데 수정되어 있는 거 같네요 ㅋㅋㅋ 굿
| public void delete(Member member, Long todoId) { | ||
| public void deleteOptionalTodo(Member member, Long roundId, Long todoId) { | ||
| Round round = findRoundById(roundId); | ||
| RoundOfMember roundOfMember = round.findRoundOfMemberBy(member); | ||
| OptionalTodo todo = findOptionalTodoById(todoId); |
There was a problem hiding this comment.
p3: 여기서도 위와 비슷한 이유로, 투두를 Repository에서 조회할 필요 없이 바로 ROM을 통해 조회+제거 작업을 진행할 수 있을 것 같아요.
+) 서비스의 update 메서드에서는 명시적으로 드러나 있는 validateTodoOwner() 검증이, delete 메서드에서는 드러나 있지 않아서 '삭제 시에는 작성자 검증이 이뤄지고 있지 않나??' 하는 의문이 생기기도 했네요. 만약 수정하게 된다면 코드의 일관성까지 챙길 수 있을 것 같아요.
|
|
||
| //when | ||
| Long id = todoService.create(member, request); | ||
| todoService.createNecessaryTodo(member, round.getId(), request); | ||
|
|
||
| //then |
There was a problem hiding this comment.
p2: then 절에 검증이 빠져 있네요. round.necessaryToDoContent가 의도한대로 잘 생성되어 있는지 검증하면 어떨까요??
There was a problem hiding this comment.
검증이 쉽지 않네요 일단 doesNotThrow로 했습니다
| TodoCreateRequest request = new TodoCreateRequest( | ||
| Boolean.parseBoolean(isNecessary), | ||
| Long.parseLong(roundId), | ||
| content | ||
| ); |
There was a problem hiding this comment.
p3: 이건 개행이 없는 게 더 깔끔해 보이는데 어떻게 생각하시나요??
kevstevie
force-pushed
the
BE/refactor/234
branch
from
73b2a7a
to
e472da0
Compare
😋 작업한 내용
투두 api, 로직 수정 및 테스트 수정
🙏 PR Point
일단 오늘은 여기까지입니다..
api 변경사항은 내일 적겠습니다..
변경된 api나 로직 관련해서 리뷰해주세요
👍 관련 이슈