step1 - 레거시 코드 리팩토링 리뷰요청 드립니다. (#812)

* feat: Answers 일급 컬렉션 적용

* feat: Question - validateOwner()

* feat: Question delete() & validateOwner TestCode

* refactor: Question 캡슐화

* refactor: 구조 개선

* refactor: Answer 클래스 수정(delete 메소드 역할 분리)

* refactor: Answers 클래스 개선(테스트코드 AnswerTest로 확인 가능)

* test: Answer test 추가

* refactor: Question 클래스 수정

* refactor: Question 클래스 객체 분리

* refactor: 공통 Delete 전용 BaseEntity 생성

* refactor: Answer 클래스에도 공통 delete BaseEntity 적용

* style: 불필요 로직 삭제

* refactor: DeleteHistory 정적 팩토리 메소드 추가 - 적용

Author: qwer920414-ctrl

src/main/java/nextstep/qna/domain/Answer.java

@@ -1,26 +1,19 @@
 package nextstep.qna.domain;
 
+import nextstep.qna.CannotDeleteException;
 import nextstep.qna.NotFoundException;
 import nextstep.qna.UnAuthorizedException;
 import nextstep.users.domain.NsUser;
 
 import java.time.LocalDateTime;
 
-public class Answer {
-    private Long id;
-
+public class Answer extends DeletableBaseEntity {
     private NsUser writer;
 
     private Question question;
 
     private String contents;
 
-    private boolean deleted = false;
-
-    private LocalDateTime createdDate = LocalDateTime.now();
-
-    private LocalDateTime updatedDate;
-
     public Answer() {
     }
 
@@ -29,12 +22,12 @@ public Answer(NsUser writer, Question question, String contents) {
     }
 
     public Answer(Long id, NsUser writer, Question question, String contents) {
-        this.id = id;
-        if(writer == null) {
+        super(id);
+        if (writer == null) {
             throw new UnAuthorizedException();
         }
 
-        if(question == null) {
+        if (question == null) {
             throw new NotFoundException();
         }
 
@@ -43,19 +36,6 @@ public Answer(Long id, NsUser writer, Question question, String contents) {
         this.contents = contents;
     }
 
-    public Long getId() {
-        return id;
-    }
-
-    public Answer setDeleted(boolean deleted) {
-        this.deleted = deleted;
-        return this;
-    }
-
-    public boolean isDeleted() {
-        return deleted;
-    }
-
     public boolean isOwner(NsUser writer) {
         return this.writer.equals(writer);
     }
@@ -64,10 +44,6 @@ public NsUser getWriter() {
         return writer;
     }
 
-    public String getContents() {
-        return contents;
-    }
-
     public void toQuestion(Question question) {
         this.question = question;
     }
@@ -76,4 +52,19 @@ public void toQuestion(Question question) {
     public String toString() {
         return "Answer [id=" + getId() + ", writer=" + writer + ", contents=" + contents + "]";
     }
+
+    public void delete(NsUser loginUser) throws CannotDeleteException {
+        validateDeletableUser(loginUser);
+        delete();
+    }
+
+    private void validateDeletableUser(NsUser loginUser) throws CannotDeleteException {
+        if (!isOwner(loginUser)) {
+            throw new CannotDeleteException("다른 사람이 쓴 답변이 있어 삭제할 수 없습니다.");
+        }
+    }
+
+    public DeleteHistory deleteHistory() {
+        return DeleteHistory.from(ContentType.ANSWER, getId(), getWriter(), LocalDateTime.now());
+    }
 }

src/main/java/nextstep/qna/domain/Answers.java

@@ -0,0 +1,38 @@
+package nextstep.qna.domain;
+
+import nextstep.qna.CannotDeleteException;
+import nextstep.users.domain.NsUser;
+
+import java.util.ArrayList;
+import java.util.List;
+
+public class Answers {
+
+    private List<Answer> answers;
+
+    public Answers() {
+        this(new ArrayList<>());
+    }
+
+    public Answers(List<Answer> answers) {
+        this.answers = new ArrayList<>(answers);
+    }
+
+    public void add(Answer answer) {
+        answers.add(answer);
+    }
+
+    public void deleteAll(NsUser loginUser) throws CannotDeleteException {
+        for (Answer answer : answers) {
+            answer.delete(loginUser);
+        }
+    }
+
+    public List<DeleteHistory> toDeleteHistories() {
+        List<DeleteHistory> deleteHistories = new ArrayList<>();
+        for (Answer answer : answers) {
+            deleteHistories.add(answer.deleteHistory());
+        }
+        return deleteHistories;
+    }
+}

src/main/java/nextstep/qna/domain/DeletableBaseEntity.java

@@ -0,0 +1,32 @@
+package nextstep.qna.domain;
+
+import java.time.LocalDateTime;
+
+public abstract class DeletableBaseEntity {
+
+    private Long id;
+    private boolean deleted = false;
+
+    private LocalDateTime createdDate = LocalDateTime.now();
+
+    private LocalDateTime updatedDate;
+
+    protected DeletableBaseEntity() {
+    }
+
+    public DeletableBaseEntity(Long id) {
+        this.id = id;
+    }
+
+    public Long getId() {
+        return id;
+    }
+
+    protected void delete() {
+        this.deleted = true;
+    }
+
+    public boolean isDeleted() {
+        return deleted;
+    }
+}

src/main/java/nextstep/qna/domain/DeleteHistory.java

@@ -19,13 +19,17 @@ public class DeleteHistory {
     public DeleteHistory() {
     }
 
-    public DeleteHistory(ContentType contentType, Long contentId, NsUser deletedBy, LocalDateTime createdDate) {
+    private DeleteHistory(ContentType contentType, Long contentId, NsUser deletedBy, LocalDateTime createdDate) {
         this.contentType = contentType;
         this.contentId = contentId;
         this.deletedBy = deletedBy;
         this.createdDate = createdDate;
     }
 
+    public static DeleteHistory from(ContentType contentType, Long contentId, NsUser deletedBy, LocalDateTime createdDate) {
+        return new DeleteHistory(contentType, contentId, deletedBy, LocalDateTime.now());
+    }
+
     @Override
     public boolean equals(Object o) {
         if (this == o) return true;

src/main/java/nextstep/qna/domain/Question.java

@@ -1,62 +1,27 @@
 package nextstep.qna.domain;
 
+import nextstep.qna.CannotDeleteException;
 import nextstep.users.domain.NsUser;
 
 import java.time.LocalDateTime;
 import java.util.ArrayList;
 import java.util.List;
 
-public class Question {
-    private Long id;
-
-    private String title;
-
-    private String contents;
+public class Question extends DeletableBaseEntity {
+    private QuestionContent content;
 
     private NsUser writer;
 
-    private List<Answer> answers = new ArrayList<>();
-
-    private boolean deleted = false;
-
-    private LocalDateTime createdDate = LocalDateTime.now();
-
-    private LocalDateTime updatedDate;
-
-    public Question() {
-    }
+    private Answers answers = new Answers();
 
     public Question(NsUser writer, String title, String contents) {
         this(0L, writer, title, contents);
     }
 
     public Question(Long id, NsUser writer, String title, String contents) {
-        this.id = id;
+        super(id);
         this.writer = writer;
-        this.title = title;
-        this.contents = contents;
-    }
-
-    public Long getId() {
-        return id;
-    }
-
-    public String getTitle() {
-        return title;
-    }
-
-    public Question setTitle(String title) {
-        this.title = title;
-        return this;
-    }
-
-    public String getContents() {
-        return contents;
-    }
-
-    public Question setContents(String contents) {
-        this.contents = contents;
-        return this;
+        this.content = new QuestionContent(title, contents);
     }
 
     public NsUser getWriter() {
@@ -72,21 +37,27 @@ public boolean isOwner(NsUser loginUser) {
         return writer.equals(loginUser);
     }
 
-    public Question setDeleted(boolean deleted) {
-        this.deleted = deleted;
-        return this;
+    private void validateOwner(NsUser loginUser) throws CannotDeleteException {
+        if (!isOwner(loginUser)) {
+            throw new CannotDeleteException("질문을 삭제할 권한이 없습니다.");
+        }
     }
 
-    public boolean isDeleted() {
-        return deleted;
+    public DeleteHistory deleteHistory() {
+        return DeleteHistory.from(ContentType.QUESTION, getId(), writer, LocalDateTime.now());
     }
 
-    public List<Answer> getAnswers() {
-        return answers;
+    public void delete(NsUser loginUser) throws CannotDeleteException {
+        validateOwner(loginUser);
+        delete();
+
+        answers.deleteAll(loginUser);
     }
 
-    @Override
-    public String toString() {
-        return "Question [id=" + getId() + ", title=" + title + ", contents=" + contents + ", writer=" + writer + "]";
+    public List<DeleteHistory> toDeleteHistories() {
+        List<DeleteHistory> deleteHistories = new ArrayList<>();
+        deleteHistories.add(deleteHistory());
+        deleteHistories.addAll(answers.toDeleteHistories());
+        return deleteHistories;
     }
 }

src/main/java/nextstep/qna/domain/QuestionContent.java

@@ -0,0 +1,11 @@
+package nextstep.qna.domain;
+
+public class QuestionContent {
+    private String title;
+    private String contents;
+
+    public QuestionContent(String title, String contents) {
+        this.title = title;
+        this.contents = contents;
+    }
+}

src/main/java/nextstep/qna/service/QnAService.java

@@ -8,7 +8,6 @@
 import org.springframework.transaction.annotation.Transactional;
 
 import javax.annotation.Resource;
-import java.time.LocalDateTime;
 import java.util.ArrayList;
 import java.util.List;
 
@@ -26,24 +25,9 @@ public class QnAService {
     @Transactional
     public void deleteQuestion(NsUser loginUser, long questionId) throws CannotDeleteException {
         Question question = questionRepository.findById(questionId).orElseThrow(NotFoundException::new);
-        if (!question.isOwner(loginUser)) {
-            throw new CannotDeleteException("질문을 삭제할 권한이 없습니다.");
-        }
+        question.delete(loginUser);
 
-        List<Answer> answers = question.getAnswers();
-        for (Answer answer : answers) {
-            if (!answer.isOwner(loginUser)) {
-                throw new CannotDeleteException("다른 사람이 쓴 답변이 있어 삭제할 수 없습니다.");
-            }
-        }
-
-        List<DeleteHistory> deleteHistories = new ArrayList<>();
-        question.setDeleted(true);
-        deleteHistories.add(new DeleteHistory(ContentType.QUESTION, questionId, question.getWriter(), LocalDateTime.now()));
-        for (Answer answer : answers) {
-            answer.setDeleted(true);
-            deleteHistories.add(new DeleteHistory(ContentType.ANSWER, answer.getId(), answer.getWriter(), LocalDateTime.now()));
-        }
+        List<DeleteHistory> deleteHistories = question.toDeleteHistories();
         deleteHistoryService.saveAll(deleteHistories);
     }
 }

src/test/java/nextstep/qna/domain/AnswerTest.java

@@ -1,8 +1,43 @@
 package nextstep.qna.domain;
 
+import nextstep.qna.CannotDeleteException;
+import nextstep.users.domain.NsUser;
 import nextstep.users.domain.NsUserTest;
+import org.junit.jupiter.api.Test;
+
+import java.time.LocalDateTime;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
 public class AnswerTest {
     public static final Answer A1 = new Answer(NsUserTest.JAVAJIGI, QuestionTest.Q1, "Answers Contents1");
     public static final Answer A2 = new Answer(NsUserTest.SANJIGI, QuestionTest.Q1, "Answers Contents2");
+
+    @Test
+    void delete_다른사람이_쓴_답변이_있으면_삭제불가() {
+        NsUser user = NsUserTest.JAVAJIGI;
+        Answer answer = AnswerTest.A2;
+        assertThatThrownBy(() -> answer.delete(user))
+                .isInstanceOf(CannotDeleteException.class);
+    }
+
+    @Test
+    void delete_성공() throws CannotDeleteException {
+        NsUser user = NsUserTest.JAVAJIGI;
+        Answer answer = AnswerTest.A1;
+
+        answer.delete(user);
+
+        assertThat(answer.isDeleted()).isTrue();
+    }
+
+    @Test
+    void deleteHistory_생성() {
+        Answer answer = AnswerTest.A1;
+
+        DeleteHistory history = answer.deleteHistory();
+
+        assertThat(history).isEqualTo(DeleteHistory.from(ContentType.ANSWER, answer.getId(), answer.getWriter(), LocalDateTime.now()));
+    }
 }