🚀 1단계 - 레거시 코드 리팩터링

README.md

@@ -1,4 +1,8 @@
 # 학습 관리 시스템(Learning Management System)
+
+## 단계별 문서
+- [🚀 1단계 - 레거시 코드 리팩터링](./docs/01-refactoring.md)
+
 ## 진행 방법
 * 학습 관리 시스템의 수강신청 요구사항을 파악한다.
 * 요구사항에 대한 구현을 완료한 후 자신의 github 아이디에 해당하는 브랜치에 Pull Request(이하 PR)를 통해 코드 리뷰 요청을 한다.

docs/01-refactoring.md

@@ -0,0 +1,37 @@
+# 🚀 1단계 - 레거시 코드 리팩터링
+***
+
+## 코드 리뷰
+> PR 링크 :
+
+## 나의 학습 목표
+
+### 1. TDD 사이클을 의식하며 기능을 구현한다. 
+- `실패 → 성공 → 리팩터링` 과정으로 작업한다.
+- 습관처럼 이 과정을 지나치면, 다시 돌아와 사이클을 반복한다.
+- TDD 사이클 습관을 만들자.
+
+### 2. 도메인에 특화된 이름을 짓는다.
+- AI에게 추천 받는 습관을 만들자.
+
+### 3. 반복된 피드백은 피하자.
+- 같은 피드백을 반복하지 않도록, 체크리스트를 만들어 점검하자.
+
+## 질문 삭제하기 요구사항
+
+- [x] 질문 데이터를 완전히 삭제하는 것이 아니라 데이터의 상태를 삭제 상태(deleted - boolean type)로 변경한다.
+- [x] 로그인 사용자와 질문한 사람이 같은 경우 삭제 가능하다.
+- [x] 답변이 없는 경우 삭제가 가능하다.
+- [x] 질문자와 답변 글의 모든 답변자가 같은 경우 삭제가 가능하다.
+- [x] 질문을 삭제할 때 답변 또한 삭제해야 하며, 답변의 삭제 또한 삭제 상태(deleted)를 변경한다.
+- [x] 질문자와 답변자가 다른 경우 답변을 삭제할 수 없다.
+- [x] 질문과 답변 삭제 이력에 대한 정보를 DeleteHistory를 활용해 남긴다.
+
+## 리팩터링 요구사항
+
+- [x] nextstep.qna.service.QnaService의 deleteQuestion()는 앞의 질문 삭제 기능을 구현한 코드이다.
+  이 메소드는 단위 테스트하기 어려운 코드와 단위 테스트 가능한 코드가 섞여 있다.
+- [x] QnaService의 deleteQuestion() 메서드에 단위 테스트 가능한 코드(핵심 비지니스 로직)를 도메인 모델 객체에 구현한다.
+- [x] QnaService의 비지니스 로직을 도메인 모델로 이동하는 리팩터링을 진행할 때 TDD로 구현한다.
+- [x] QnaService의 deleteQuestion() 메서드에 대한 단위 테스트는 src/test/java 폴더 nextstep.qna.service.QnaServiceTest이다.
+  도메인 모델로 로직을 이동한 후에도 QnaServiceTest의 모든 테스트는 통과해야 한다.

docs/check-list.md

@@ -0,0 +1,12 @@
+# ✔️ PR 전 확인해야될 목록
+***
+
+- 객체지향 생활체조 원칙을 준수하였는가 ?
+- TDD 사이클로 구현하였는가 ?
+- TDD 사이클 단위로 커밋하였는가 ?
+- 객체에게 상태를 노출시키지 않고 책임을 맡겼는가 ?
+- 테슽으는 객체의 책임과 결과만 검증했는가 ?
+- 불변성을 최대한 유지했는가 ?
+- 도메인 용어로 네이밍 했는가 ?
+- 협력 구조가 자연스럽게 읽히는가 ?
+- 같은 피드백을 반복하지는 않았는가 ?

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

@@ -1,5 +1,6 @@
 package nextstep.qna.domain;
 
+import nextstep.qna.CannotDeleteException;
 import nextstep.qna.NotFoundException;
 import nextstep.qna.UnAuthorizedException;
 import nextstep.users.domain.NsUser;
@@ -30,11 +31,11 @@ public Answer(NsUser writer, Question question, String contents) {
 
     public Answer(Long id, NsUser writer, Question question, String contents) {
         this.id = id;
-        if(writer == null) {
+        if (writer == null) {
             throw new UnAuthorizedException();
         }
 
-        if(question == null) {
+        if (question == null) {
             throw new NotFoundException();
         }
 
@@ -52,6 +53,10 @@ public Answer setDeleted(boolean deleted) {
         return this;
     }
 
+    public void updateDeleted(){
+        this.deleted = true;
+    }
+
     public boolean isDeleted() {
         return deleted;
     }
@@ -72,6 +77,12 @@ public void toQuestion(Question question) {
         this.question = question;
     }
 
+    public void validateAnswerOwner(NsUser loginUser) throws CannotDeleteException {
+        if (!this.isOwner(loginUser)) {
+            throw new CannotDeleteException("다른 사람이 쓴 답변이 있어 삭제할 수 없습니다.");
+        }
+    }
+
     @Override
     public String toString() {
         return "Answer [id=" + getId() + ", writer=" + writer + ", contents=" + contents + "]";

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

@@ -0,0 +1,34 @@
+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 Answers {
+
+    private List<Answer> answers = new ArrayList<>();
+
+    public void add(Answer answer) {
+        this.answers.add(answer);
+    }
+
+    public List<Answer> getAnswers() {
+        return answers;
+    }
+
+    public void validateAnswerOwner(NsUser loginUser) throws CannotDeleteException {
+        for (Answer answer : this.answers) {
+            answer.validateAnswerOwner(loginUser);
+        }
+    }
+
+    public void addDeleteAnswerHistory(List<DeleteHistory> deleteHistories) {
+        for (Answer answer : this.answers) {
+            answer.updateDeleted();
+            deleteHistories.add(new DeleteHistory(ContentType.ANSWER, answer.getId(), answer.getWriter(), LocalDateTime.now()));
+        }
+    }
+}

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

@@ -1,5 +1,6 @@
 package nextstep.qna.domain;
 
+import nextstep.qna.CannotDeleteException;
 import nextstep.users.domain.NsUser;
 
 import java.time.LocalDateTime;
@@ -15,7 +16,7 @@ public class Question {
 
     private NsUser writer;
 
-    private List<Answer> answers = new ArrayList<>();
+    private Answers answers = new Answers();
 
     private boolean deleted = false;
 
@@ -65,7 +66,7 @@ public NsUser getWriter() {
 
     public void addAnswer(Answer answer) {
         answer.toQuestion(this);
-        answers.add(answer);
+        this.answers.add(answer);
     }
 
     public boolean isOwner(NsUser loginUser) {
@@ -82,11 +83,42 @@ public boolean isDeleted() {
     }
 
     public List<Answer> getAnswers() {
-        return answers;
+        return answers.getAnswers();
+    }
+
+    public void validateDeletable(NsUser loginUser) throws CannotDeleteException {
+        validateOwner(loginUser);
+        answers.validateAnswerOwner(loginUser);
+    }
+
+    private void validateOwner(NsUser loginUser) throws CannotDeleteException {
+        if (!this.isOwner(loginUser)) {
+            throw new CannotDeleteException("질문을 삭제할 권한이 없습니다.");
+        }
+    }
+
+    public List<DeleteHistory> delete(long questionId) {
+        // todo 2개의 역할을 하고 있다고 느껴집니다. deleted 상태를 Setter 사용하지 않고 어떻게 처리해야될지 고민입니다.
+        updateDeleted();
+
+        List<DeleteHistory> deleteHistories = new ArrayList<>();
+        deleteHistories.add(new DeleteHistory(ContentType.QUESTION, questionId, this.writer, LocalDateTime.now()));
+        addDeleteAnswerHistory(deleteHistories);
+
+        return deleteHistories;
+    }
+
+    private void addDeleteAnswerHistory(List<DeleteHistory> deleteHistories) {
+        this.answers.addDeleteAnswerHistory(deleteHistories);
+    }
+
+    private void updateDeleted() {
+        this.deleted = true;
     }
 
     @Override
     public String toString() {
         return "Question [id=" + getId() + ", title=" + title + ", contents=" + contents + ", writer=" + writer + "]";
     }
+
 }

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

@@ -8,8 +8,6 @@
 import org.springframework.transaction.annotation.Transactional;
 
 import javax.annotation.Resource;
-import java.time.LocalDateTime;
-import java.util.ArrayList;
 import java.util.List;
 
 @Service("qnaService")
@@ -26,24 +24,8 @@ 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("질문을 삭제할 권한이 없습니다.");
-        }
-
-        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()));
-        }
+        question.validateDeletable(loginUser);
+        List<DeleteHistory> deleteHistories = question.delete(questionId);
         deleteHistoryService.saveAll(deleteHistories);
     }
 }

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

@@ -1,8 +1,26 @@
 package nextstep.qna.domain;
 
+import nextstep.qna.CannotDeleteException;
 import nextstep.users.domain.NsUserTest;
+import org.junit.jupiter.api.Test;
+
+import static org.assertj.core.api.Assertions.*;
+import static org.junit.jupiter.api.Assertions.*;
 
 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 다른_사용자가_작성한_답변이_존재하면_에러발생(){
+        assertThrows(CannotDeleteException.class, () -> {
+            A1.validateAnswerOwner(NsUserTest.SANJIGI);
+        });
+    }
+
+    @Test
+    void 답변삭제(){
+        A1.updateDeleted();
+        assertThat(A1.isDeleted()).isTrue();
+    }
 }

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

@@ -1,8 +1,32 @@
 package nextstep.qna.domain;
 
+import nextstep.qna.CannotDeleteException;
 import nextstep.users.domain.NsUserTest;
+import org.junit.jupiter.api.Test;
+
+import java.util.List;
+
+import static org.assertj.core.api.Assertions.*;
+import static org.junit.jupiter.api.Assertions.*;
 
 public class QuestionTest {
     public static final Question Q1 = new Question(NsUserTest.JAVAJIGI, "title1", "contents1");
     public static final Question Q2 = new Question(NsUserTest.SANJIGI, "title2", "contents2");
+
+    @Test
+    void 글쓴이가_로그인한_유저와_같지않으면_에러발생(){
+        assertThrows(CannotDeleteException.class, () -> {
+            Q1.validateDeletable(NsUserTest.SANJIGI);
+        });
+    }
+
+    @Test
+    void 질문_답글_삭제_목록_조회(){
+        List<DeleteHistory> deleteHistories = Q1.delete(1L);
+        assertThat(deleteHistories).hasSize(1);
+    }
+
+
+
+
 }