[REFACTOR] 투표 도메인 리팩터링 #727

aiaiaiai1 · 2023-10-09T13:41:38Z

🔥 연관 이슈

close: #514

📝 작업 요약

Vote 도메인을 리펙터링 하였습니다.
기존에는 Post가 Vote를 생성하였는데 VoteService에서 생성하도록 하여 의존도를 분리했습니다.
Post에서 투표에 대한 검증 로직을 VoteService와 Vote에 위임하여 예외처리 책임을 분리했습니다.
VoteSerivce의 테스트 코드를 추가하였습니다.

⏰ 소요 시간

3시간

🔎 작업 상세 설명

🌟 논의 사항

- 투표시 마감기간, 선택지 옵션 검증로직 serivce계층에 위임 - 투표시 작성자 검증 로직 Vote 도메인에 위임

github-actions · 2023-10-09T13:44:49Z

Test Results

382 tests 382 ✔️ 26s ⏱️
119 suites     0 💤
119 files     0 ❌

Results for commit 0f56c56.

♻️ This comment has been updated with latest results.

woo-chang

안녕하세요 루쿠 ! 다즐입니다 ㅎㅎ

의존성을 분리하는 로직 확인했습니다 ~
몇 가지 작은 커멘트와 질문 남겨두었으니 확인해주시면 감사합니다 🙇🏻‍♂️

woo-chang · 2023-10-11T06:18:13Z

backend/src/main/java/com/votogether/domain/vote/entity/Vote.java

@@ -37,15 +39,19 @@ public class Vote extends BaseEntity {

    @Builder
    private Vote(final Member member, final PostOption postOption) {
+        validateVotingRights(member, postOption);


생성자에게 꼼꼼한 검증은 좋은 방법이라고 생각합니다 👍

한 가지 우려되는 점은 Vote에서 Post 의존성을 가지게 되면 Vote -> Post -> PostOption -> Vote의 순환 참조가 발생할 수 있는 위험이 증가한다고 생각이 들어요.

따라서 해당 검증은 Service 계층의 비즈니스 로직에서 처리하는게 어떤지 물어보고 싶어요 :)

woo-chang · 2023-10-11T06:18:41Z

backend/src/main/java/com/votogether/domain/vote/exception/VoteExceptionType.java

@@ -6,12 +6,11 @@
 @Getter
 public enum VoteExceptionType implements ExceptionType {

-    CANNOT_VOTE_MY_POST(700, "해당 게시글 작성자는 투표할 수 없습니다."),
+    CANNOT_VOTE_MY_POST(700, "게시글 작성자는 투표할 수 없습니다."),


woo-chang · 2023-10-11T06:26:30Z

backend/src/main/java/com/votogether/domain/vote/service/VoteService.java

+        validateDeadline(post);
+        validatePostOption(post, postOption);


검증은 생성 메서드와 별도로 한 곳에 묶어두는 건 어떨까요?!

숨겨진 게시글에 대한 검증도 있으면 좋을 것 같아요 ㅎㅎ

woo-chang · 2023-10-11T06:28:19Z

backend/src/test/java/com/votogether/domain/vote/service/VoteServiceTest.java

+
+        @Test
+        @DisplayName("본인의 게시글에 투표하는 경우 예외 발생")
+        void vote1() {


팀 컨벤션에 맞게 영어 테스트명도 한글 테스트명에 알맞게 작명하면 좋을 것 같아요 ~

woo-chang · 2023-10-11T06:29:27Z

backend/src/test/java/com/votogether/domain/vote/service/VoteServiceTest.java

+                    .isInstanceOf(BadRequestException.class)
+                    .hasMessage("이미 참여한 투표입니다.");
+        }
+


해당 선택지는 아니지만 해당 게시글의 다른 선택지에 투표한 상황에 대한 테스트와 숨겨진 게시글을 투표한 상황에 대한 테스트도 있으면 더욱 좋을 것 같습니다 🤓

woo-chang · 2023-10-11T06:29:57Z

backend/src/test/java/com/votogether/domain/vote/service/VoteServiceTest.java

+    class vote {
+
+        @Test
+        @DisplayName("투표 성공.")


성공 메서드명에서만 .을 넣으신 이유가 궁금합니다 !

tjdtls690

수고하셨습니다 :)
몇 가지 코멘트 남겼습니다!

tjdtls690 · 2023-10-16T06:47:03Z

backend/src/main/java/com/votogether/domain/vote/service/VoteService.java

+        final Vote vote = Vote.builder()
+                .member(member)
+                .postOption(postOption)
+                .build();
+        postOption.addVote(vote);
+        return vote;


createVote메서드에선 postOption 셋팅을 제외한 Vote를 만드는 로직만 남겨두고, 양방향 관계를 맺어주는 건 이미 postOption.addVote(vote) 여기서 해주고 있으니 차라리 밑의 예시처럼 구현하는 것은 어떨까요?

private Vote createVote(final Member member) { return Vote.builder() .member(member) .build(); }

그리고 vote 메서드에서

postOption.addVote(vote);

이 코드를 진행하는 형식입니다.

createVote 메서드의 파라미터 수도 줄일 수 있고, 코드의 흐름을 파악하기 더 수월할 것 같아요 :)

tjdtls690 · 2023-10-16T06:47:52Z

backend/src/main/java/com/votogether/domain/vote/service/VoteService.java

@@ -72,8 +94,8 @@ public void changeVote(
                .orElseThrow(() -> new NotFoundException(PostOptionExceptionType.NOT_FOUND));

        voteRepository.delete(originVote);
-        final Vote vote = post.makeVote(member, newPostOption);
-        voteRepository.save(vote);
+        final Vote newVote = createVote(member, post, newPostOption);


이 부분도 위의 리뷰 내용과 동일하게 적용 될 것 같습니다.

tjdtls690 · 2023-10-16T06:52:25Z

backend/src/test/java/com/votogether/domain/vote/service/VoteServiceTest.java

+
+            // then
+            int count = voteRepository.countByMember(member);
+            assertThat(count).isEqualTo(1);


.isEqualTo(1); 대신 isOne(); 은 어떨까요

tjdtls690 · 2023-10-17T01:59:30Z

backend/src/main/java/com/votogether/domain/vote/service/VoteService.java

        final PostOption postOption = postOptionRepository.findById(postOptionId)
                .orElseThrow(() -> new NotFoundException(PostOptionExceptionType.NOT_FOUND));

-        final Vote vote = post.makeVote(member, postOption);
+        validateAlreadyVoted(member, post);
+        final Vote vote = createVote(member, post, postOption);
        voteRepository.save(vote);


이 부분은 생략해도 변경 감지에 의해 vote가 자동으로 insert 될 것 같아요

jeomxon

여러 번 읽었는데 남겨야할 리뷰를 찾기 어렵네요..^^
다즐과 아벨의 코멘트 반영해주시면 될 것 같아요!
고생하셨습니다 루쿠 :)

aiaiaiai1 added 5 commits October 9, 2023 16:35

refactor: 사용하지 않는 메서드 삭제

3c78b11

refactor: (#514) Post 객체에서 vote 의존성 제거

91bb5aa

- 투표시 마감기간, 선택지 옵션 검증로직 serivce계층에 위임 - 투표시 작성자 검증 로직 Vote 도메인에 위임

test: (#514) VoteService, Vote 테스트 코드 추가

def5a4f

refactor: (#514) docs 개행 수정

1f91424

Merge branch 'dev' into refactor/#514

7735524

aiaiaiai1 requested review from jeomxon, woo-chang and tjdtls690 October 9, 2023 13:41

aiaiaiai1 self-assigned this Oct 9, 2023

aiaiaiai1 added BE 🐳 인프라 🌀 우선 순위 낮음 labels Oct 9, 2023

aiaiaiai1 added this to the 6차 데모데이 milestone Oct 9, 2023

aiaiaiai1 removed the 🐳 인프라 label Oct 9, 2023

test: (#514) 테스트 실패 코드 수정

0f56c56

woo-chang requested changes Oct 11, 2023

View reviewed changes

tjdtls690 requested changes Oct 17, 2023

View reviewed changes

Gilpop8663 modified the milestones: 6차 데모데이, 최종 데모데이 Oct 19, 2023

woo-chang removed this from the 최종 데모데이 milestone Oct 19, 2023

jeomxon approved these changes Nov 3, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[REFACTOR] 투표 도메인 리팩터링 #727

[REFACTOR] 투표 도메인 리팩터링 #727

aiaiaiai1 commented Oct 9, 2023

github-actions bot commented Oct 9, 2023 •

edited

Loading

woo-chang left a comment

woo-chang Oct 11, 2023

woo-chang Oct 11, 2023

woo-chang Oct 11, 2023

woo-chang Oct 11, 2023

woo-chang Oct 11, 2023

woo-chang Oct 11, 2023

woo-chang Oct 11, 2023

tjdtls690 left a comment

tjdtls690 Oct 16, 2023

tjdtls690 Oct 16, 2023

tjdtls690 Oct 16, 2023

tjdtls690 Oct 17, 2023

jeomxon left a comment

		validateDeadline(post);
		validatePostOption(post, postOption);

[REFACTOR] 투표 도메인 리팩터링 #727

Are you sure you want to change the base?

[REFACTOR] 투표 도메인 리팩터링 #727

Conversation

aiaiaiai1 commented Oct 9, 2023

🔥 연관 이슈

📝 작업 요약

⏰ 소요 시간

🔎 작업 상세 설명

🌟 논의 사항

github-actions bot commented Oct 9, 2023 • edited Loading

Test Results

woo-chang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tjdtls690 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeomxon left a comment

Choose a reason for hiding this comment

github-actions bot commented Oct 9, 2023 •

edited

Loading