🚀 1단계 - 레거시 코드 리팩터링 (#790)

* docs : 기능 요구사항 문서 작성

* docs : 나의 학습 목표, PR전 확인해야할 목록 작성(호성님이 작성한 내용에 감명 받아 저도 관리하려고합니다.)

* refactor : Question 객체에 질문 삭제 권한 체크 메서드 생성 및 테스트 코드 작성, Answer 객체에 삭제 권한 체크 메서드 생성 및 테스트 코드 작성

* refactor : Question 객체의 인스턴스 변수(answer) 활용해 직접 Answer 객체의 validateAnswerOwner 메서드 호출

* refactor : 질문, 답변 삭제 로직 Question 객체로 이동(Deleted 값을 변경하는 로직이 포함되어 있어서 어떻게 처리하면 좋을지 고민중) 및 테스트 코드 작성

* refactor : Question 클래스의 List<Answer> 변수 포장

* docs : 헷갈리는 내용 Todo 추가

* refactor : 공동으로 사용되는 createdDate, updatedDate, delted를 BaseModel로 분리

* refactor : title, contents 객체 포장

* refactor : delete() 메서드 파라미터 제거 및 테스트 코드 수정

* refactor : 검증하는 로직과 상태 변경하는 로직을 함께 구현,
상태를 변경하는 메서드와 삭제 내역을 남기는 history를 분리

* feat : Answers 테스트 코드 생성
refactor : answer 테스트 코드 추가, Question 테스트 코드 내부 수정

* refactor : updateDeleted() 메서드 private으로 변경, 호출하는 위치 변경 및 테스트 코드 수정

* refactor : BaseModel 클래스의 인스턴스 변수 private로 변경, 필요한 메서드 protected로 선언 패키지 내에서 사용 가능하도록 생성

* refactor : BaseModel 클래스명 변경
## AI의 추천을 받아 적합한 이름을 선택했습니다.

### 추천 클래스명
1. DeletableModel
2. SoftDeletableModel
3. AuditableModel

해당 클래스는 soft delete 정책을 따르기 때문에 2번의 SoftDeletableModel로 선택

* refactor : Title, Contents 클래스 제거 후 공용으로 사용할 QuestionBody 클래스 생성

Author: chanani

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 링크 : [#790](https://github.com/next-step/java-lms/pull/790)
+
+## 나의 학습 목표
+
+### 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,40 +1,37 @@
 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 {
+public class Answer extends SoftDeletableModel {
     private Long id;
 
     private NsUser writer;
 
     private Question question;
 
-    private String contents;
-
-    private boolean deleted = false;
-
-    private LocalDateTime createdDate = LocalDateTime.now();
-
-    private LocalDateTime updatedDate;
+    private QuestionBody contents;
 
     public Answer() {
     }
 
     public Answer(NsUser writer, Question question, String contents) {
-        this(null, writer, question, contents);
+        this(null, writer, question, new QuestionBody(contents));
     }
 
     public Answer(Long id, NsUser writer, Question question, String contents) {
+       this(id, writer, question, new QuestionBody(contents));
+    }
+
+    public Answer(Long id, NsUser writer, Question question, QuestionBody contents) {
         this.id = id;
-        if(writer == null) {
+        if (writer == null) {
             throw new UnAuthorizedException();
         }
 
-        if(question == null) {
+        if (question == null) {
             throw new NotFoundException();
         }
 
@@ -47,13 +44,12 @@ public Long getId() {
         return id;
     }
 
-    public Answer setDeleted(boolean deleted) {
-        this.deleted = deleted;
-        return this;
+    private void updateDeleted() {
+        deleted();
     }
 
     public boolean isDeleted() {
-        return deleted;
+        return getDeleted();
     }
 
     public boolean isOwner(NsUser writer) {
@@ -64,14 +60,18 @@ public NsUser getWriter() {
         return writer;
     }
 
-    public String getContents() {
-        return contents;
-    }
 
     public void toQuestion(Question question) {
         this.question = question;
     }
 
+    public void validateAnswerOwner(NsUser loginUser) throws CannotDeleteException {
+        if (!this.isOwner(loginUser)) {
+            throw new CannotDeleteException("다른 사람이 쓴 답변이 있어 삭제할 수 없습니다.");
+        }
+        updateDeleted();
+    }
+
     @Override
     public String toString() {
         return "Answer [id=" + getId() + ", writer=" + writer + ", contents=" + contents + "]";

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

@@ -0,0 +1,36 @@
+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 List<DeleteHistory> addDeleteAnswerHistory() {
+        List<DeleteHistory> deleteHistories = new ArrayList<>();
+        for (Answer answer : this.answers) {
+            deleteHistories.add(new DeleteHistory(ContentType.ANSWER, answer.getId(), answer.getWriter(), LocalDateTime.now()));
+        }
+        return deleteHistories;
+    }
+}

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

@@ -1,27 +1,22 @@
 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 {
+public class Question extends SoftDeletableModel {
     private Long id;
 
-    private String title;
+    private QuestionBody title;
 
-    private String contents;
+    private QuestionBody contents;
 
     private NsUser writer;
 
-    private List<Answer> answers = new ArrayList<>();
-
-    private boolean deleted = false;
-
-    private LocalDateTime createdDate = LocalDateTime.now();
-
-    private LocalDateTime updatedDate;
+    private Answers answers = new Answers();
 
     public Question() {
     }
@@ -31,6 +26,10 @@ public Question(NsUser writer, String title, String contents) {
     }
 
     public Question(Long id, NsUser writer, String title, String contents) {
+        this(id, writer, new QuestionBody(title), new QuestionBody(contents));
+    }
+
+    public Question(Long id, NsUser writer, QuestionBody title, QuestionBody contents) {
         this.id = id;
         this.writer = writer;
         this.title = title;
@@ -41,52 +40,56 @@ public Long getId() {
         return id;
     }
 
-    public String getTitle() {
-        return title;
+    public NsUser getWriter() {
+        return writer;
     }
 
-    public Question setTitle(String title) {
-        this.title = title;
-        return this;
+    public void addAnswer(Answer answer) {
+        answer.toQuestion(this);
+        this.answers.add(answer);
     }
 
-    public String getContents() {
-        return contents;
+    public boolean isOwner(NsUser loginUser) {
+        return writer.equals(loginUser);
     }
 
-    public Question setContents(String contents) {
-        this.contents = contents;
-        return this;
+    public boolean isDeleted() {
+        return getDeleted();
     }
 
-    public NsUser getWriter() {
-        return writer;
+    private void validateOwner(NsUser loginUser) throws CannotDeleteException {
+        if (!this.isOwner(loginUser)) {
+            throw new CannotDeleteException("질문을 삭제할 권한이 없습니다.");
+        }
+        updateDeleted();
     }
 
-    public void addAnswer(Answer answer) {
-        answer.toQuestion(this);
-        answers.add(answer);
+    public void delete(NsUser loginUser) throws CannotDeleteException {
+        validateOwner(loginUser);
+        answers.validateAnswerOwner(loginUser);
     }
 
-    public boolean isOwner(NsUser loginUser) {
-        return writer.equals(loginUser);
-    }
+    public List<DeleteHistory> toDeleteHistories(){
+        List<DeleteHistory> deleteHistories = new ArrayList<>();
+        deleteHistories.add(new DeleteHistory(ContentType.QUESTION, this.id, this.writer, LocalDateTime.now()));
 
-    public Question setDeleted(boolean deleted) {
-        this.deleted = deleted;
-        return this;
+        List<DeleteHistory> answerDeleteHistories = addDeleteAnswerHistory();
+        deleteHistories.addAll(answerDeleteHistories);
+
+        return deleteHistories;
     }
 
-    public boolean isDeleted() {
-        return deleted;
+    private List<DeleteHistory> addDeleteAnswerHistory() {
+        return this.answers.addDeleteAnswerHistory();
     }
 
-    public List<Answer> getAnswers() {
-        return answers;
+    private void updateDeleted() {
+        deleted();
     }
 
     @Override
     public String toString() {
         return "Question [id=" + getId() + ", title=" + title + ", contents=" + contents + ", writer=" + writer + "]";
     }
+
 }

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

@@ -0,0 +1,10 @@
+package nextstep.qna.domain;
+
+public class QuestionBody {
+
+    private String value;
+
+    public QuestionBody(String value) {
+        this.value = value;
+    }
+}

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

@@ -0,0 +1,20 @@
+package nextstep.qna.domain;
+
+import java.time.LocalDateTime;
+
+public abstract class SoftDeletableModel {
+
+    private LocalDateTime createdDate = LocalDateTime.now();
+
+    private LocalDateTime updatedDate;
+
+    private boolean deleted = false;
+
+    protected void deleted(){
+        this.deleted = true;
+    }
+
+    protected boolean getDeleted() {
+        return deleted;
+    }
+}

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,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);
     }
 }