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

selene-p · 2025-04-06T09:51:58Z

안녕하세요 🤓
1단계 구현 완료하여 PR 드립니다!
마지막 단계까지 잘 부탁드립니닷 🙇‍♀️

📌 리팩터링 내용

QnaService의 비즈니스 로직을 도메인 모델로 이동

기존 QnaService의 deleteQuestion() 메서드가 너무 많은 비즈니스 로직을 포함하고 있어 이를 도메인 모델로 분리하였습니다.
Question, Answers, Answer, DeleteHistory 도메인 클래스로 기능을 재구성하였습니다.
Question이 가지고 있는 List<Answer>를 일급컬렉션인 Answers로 한번 감쌌습니다.

Question 에서 답변 삭제 로직을 모두 담당하도록 하여 책임을 명확히 하였습니다.

delete() 메서드 내부에서 createDeleteHistory() 를 호출하도록 하여 삭제 기록 생성이 항상 보장되도록 구현하였습니다.

🧐 고민 포인트

Question과 Answer의 delete 메서드가 DeleteHistory 객체를 반환하는 것이 어색하다고 느꼈는데요ㅠ
단순히 삭제 로직을 수행하는 메서드가 데이터를 반환하는 것이 좀 어색하다는 생각이 계속 들긴 했습니다...!
그런데 만약 delete 메서드와 DeleteHistory 생성을 별도로 처리하면, 사용자가 delete 메서드를 호출할 때 DeleteHistory 생성을 빠뜨릴 수 있다는 문제(?)점이 있을 것 같아서, DeleteHistory 생성이 보장을 위해 delete 메서드 안에 구현하긴 했습니다..!

jinyoungchoi95

안녕하세요 해원님 😄

수강신청 미션의 리뷰어를 맡은 최진영입니다.

미션 잘 진행해주셨네요! 몇가지 코멘트 남겨두었으니 다음 단계 진행하시면서 함께 확인부탁드려요 :)

궁금하거나 고민이 되는 부분이 있으시다면 언제든 pr 코멘트 또는 dm으로 요청 부탁드립니다.
감사합니다 🙇‍♂️

jinyoungchoi95 · 2025-04-07T10:21:10Z

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

-        return id;
+    public DeleteHistory delete() {
+        this.deleted = true;
+        return createDeleteHistory();


네 저는 delete 처리의 결과로 DeleteHistory를 반환하는게 크게 어색해보이지는 않아요.

메서드분리까지는 과하지 않나라는 생각이 드네요 😄

jinyoungchoi95 · 2025-04-07T10:21:29Z

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

+import java.util.Collections;
+import java.util.List;
+
+public class Answers {


일급 컬랙션 잘 만들어주셨네요 👍

jinyoungchoi95 · 2025-04-07T10:22:42Z

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

+
+public class Answers {
+
+  private final List<Answer> answerList;


보유한 데이터에 대해서 List로 자료형을 표현하지 않아도 좋을 것 같아요. 관리하는 대상은 answers이고, 이미 타입에서 어떤 자료형인지가 표현이 되어있기도하구요.

자료형이 변경된다면 변수명도 같이 바뀌어야한다는 단점도 있겠네요 😄

jinyoungchoi95 · 2025-04-07T10:24:43Z

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

+        if (!answers.areAllAnswersSameWriter(writer)) {
+            throw new CannotDeleteException("다른 사람이 쓴 답변이 있어 삭제할 수 없습니다.");
+        }


답변 본인의 삭제에 대한 예외처리는 Answer 내부에서 처리하여야하지 않을까요? :)

jinyoungchoi95 · 2025-04-07T10:25:03Z

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

-            answer.setDeleted(true);
-            deleteHistories.add(new DeleteHistory(ContentType.ANSWER, answer.getId(), answer.getWriter(), LocalDateTime.now()));
-        }
+        List<DeleteHistory> deleteHistories = question.delete(loginUser);


깔끔하게 잘 처리되었네요 👍

selene-p added 12 commits April 6, 2025 16:32

refactor: Question 1차 분리

c63ee11

refactor: Answer 1차 분리

55bcced

refactor: Answers 일급 콜렉션 생성

e73b654

test: DeleteHistory 테스트코드 생성

4b552d1

test: 테스트코드 보강

fe0c63e

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

9e8a284

refactor: Exception들은 별도 패키지로 묶기

02380c3

test: NsUser 테스트코드 작성

f8a6a3f

refactor: QnaService 테스트코드 변경

b4473de

docs: 체크리스트 추가

ffc7c81

fix: 삭제 시 updatedDate는 건드리지 않는다

7f012cb

refactor: 메서드 작은 단위로 분리

ae09fb5

jinyoungchoi95 approved these changes Apr 7, 2025

View reviewed changes

jinyoungchoi95 merged commit d8925be into next-step:selene-p Apr 7, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

selene-p commented Apr 6, 2025

jinyoungchoi95 left a comment

jinyoungchoi95 Apr 7, 2025

jinyoungchoi95 Apr 7, 2025

jinyoungchoi95 Apr 7, 2025

jinyoungchoi95 Apr 7, 2025

jinyoungchoi95 Apr 7, 2025


		public class Answers {

		private final List<Answer> answerList;

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

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

Conversation

selene-p commented Apr 6, 2025

jinyoungchoi95 left a comment

Choose a reason for hiding this comment

jinyoungchoi95 Apr 7, 2025

Choose a reason for hiding this comment

jinyoungchoi95 Apr 7, 2025

Choose a reason for hiding this comment

jinyoungchoi95 Apr 7, 2025

Choose a reason for hiding this comment

jinyoungchoi95 Apr 7, 2025

Choose a reason for hiding this comment

jinyoungchoi95 Apr 7, 2025

Choose a reason for hiding this comment