Step1 - 레거시 코드 리팩토링 #809
Step1 - 레거시 코드 리팩토링 #809
Conversation
* 요구사항 토대로 질문, 답변의 행위 설정 * 설정된 행위들 TDD 구현 * 레포지토리 인터페이스 패키지 이관
* 예외객체 패키징 * 삭제이력 응답행위 질문,답변 객체에 구현 * QnADomainService 기반 도메인 로직 조합 * QnAService deleteQuestion 개선.
* 미사용 코드 제거 및 get/set 제거 * 테스트 코드 정리
javajigi
left a comment
javajigi
left a comment
There was a problem hiding this comment.
1단계 레거시 코드 리팩터링 미션 넘 구현 잘 했네요. 💯
특히 지금까지 여러 기수를 운영하며 여러 수강생의 코드를 봤는데요.
DomainService 객체를 추가한 경우는 처음 보는 것 같아 신선해어요.
바로 merge하려다 Question의 필드 수를 줄이기 위한 객체 분리를 추가로 도전해 봤으면 하는 바람으로 피드백 남겨 봅니다.
Q1. 결합도를 낮추기 위해 Question, Answer의 NsUser를 writerId로 대체했습니다. (막상 참조하고 있어도 사용하는 경우도 없고, 도메인로직인 작성자 확인도 id 면 충분하다고 판단) 다만 모든상황에서 결합도를 제거하는게 무조건 좋진 않을것 같은데. 이렇게 결합도를 제거하기전 고려해볼만한 조건들은 뭐가 있을까요?
A1. 무조건적으로 의존관계를 가져가는 것은 복잡도나 유지보수 측면에서 바람직한 것은 아니라 생각해요. 객체간의 의존관계가 밀접한 부분과 그렇지 않은 부분을 파악해 분리할 부분은 분리해 나가는 역량도 쌓아나갈 필요가 있다 생각해요. 현재 시도한 바와 같이 NsUser는 분리할 수 있겠지만 Question과 Answer는 밀접한 관계이니 의존관계를 유지하자와 같이 객체 간의 의존성이 얼마나 밀접한지는 고민해 보면 좋겠습니다. 어차피 이 또한 지속적인 설계 역량을 쌓는 수 밖에 없는 것 같아요.
Q2. 변화가능한 상태를 가진 객체는 불변객체로 다루는게 맞는가? 에 대한 고민(ex 엔티티, 도메인객체)
addAnswer를 할 경우는 질문삭제 요구사항 외에도 많을 거라는 추측.
A2. 모든 객체를 불변 객체로 유지하는데는 한계가 있다 생각해요. 물론 최대한 불변 객체를 유지하기 위해 노력할 필요는 있겠지만 무조건 이 원칙을 지켜야 한다고 생각하지 않습니다. 물론 개개인에 따라 다르겠지만 저와 같이 실용주의 노선을 걷는 입장에서는 상황에 따라 달라질 수 있다 생각해요.
다른 부분의 고민에 대해서는 민영님의 시도에 공감하는 부분이 많아 별도의 답변은 남기지 않았어요.
피드백 빠르게 반영하고 2단계 진행해 보시죠.
|
|
||
| public boolean isOwner(NsUser writer) { | ||
| return this.writer.equals(writer); | ||
| public void putOnDelete(long requesterId) { |
|
|
||
| public String getContents() { | ||
| return contents; | ||
| public DeleteHistory createAnswerDeleteHistory() { |
|
|
||
| import java.util.List; | ||
|
|
||
| public class QnADomainService { |
There was a problem hiding this comment.
새로운 DomainService를 분리한 점이 인상적임 👍
상태 값이 없다면 이 객체 또한 빈으로 등록하고 di하는 것과 현재와 같이 직접 생성하는 것 중 어느 방식이 더 나을까?
| import nextstep.qna.exception.unchecked.WrongRequestException; | ||
| import nextstep.users.domain.NsUser; | ||
|
|
||
| public class Question { |
There was a problem hiding this comment.
Question 객체는 우리가 현장에서 흔하게 접하는 인스턴스 변수가 많은 객체의 모습이다.
객체 지향 생활 체조 원칙의 다음 두 가지 원칙에 지키기 위해 노력해 본다.
- 규칙 6: 모든 엔티티를 작게 유지한다.
- 규칙 7: 3개 이상의 인스턴스 변수를 가진 클래스를 쓰지 않는다.
특히 규칙 7의 경우 지키기 힘들다 하더라도 인스턴스 변수의 수를 줄이기 위해 도전해 본다.
힌트: 상속을 통한 인스턴스 변수 줄이기, 관련 있는 인스턴스 변수를 새로운 객체로 분리하기 등 활용
| public NsUser getWriter() { | ||
| return writer; | ||
| } | ||
| public void putOnDelete(long requesterId) { |
| private NsUser writer; | ||
| private Long writerId; | ||
|
|
||
| private List<Answer> answers = new ArrayList<>(); |
There was a problem hiding this comment.
결국 Answers의 불변 vs addAnswer() 제거 중 Answers의 불변을 포기했습니다.
이 내용이 있어 일급 콜렉션으로 구현한 것으로 판단
일급 콜렉션으로 구현할 때와 하지 않아도 될 때를 구분하는 고민을 한 것에 의미가 있는 것으로 보임
* 소프트삭제 가능 추상클래스 생성 및 질문 도메인객체에 관련 내용 적용
* 소프트삭제 추상클래스 답변 도메인객체에 적용
* 소프트삭제 추상클래스 네이밍 변경
* 게시판 내용 객체 생성 * 질문 & 답변 객체에 적용
* 로그인 키페어 객체 생성 및 회원객체에 적용 * 회원객체 불필요 코드 제거
* 기본 도메인객체 추상클래스 생성 및 유저 도메인 객체에 적용 * 삭제가능 추상클래스에 id 추가 및 적용
* 기본 도메인객체 추상클래스 삭제이력 도메인 객체에 적용 * 테스트 실패로 생성/수정시간 관련 로직 외부에서 주입받게 변경
javajigi
left a comment
javajigi
left a comment
There was a problem hiding this comment.
피드백 반영 넘 잘 했네요. 💯
특별히 리팩터링하지 않아도 되는데 BaseEntity를 추가해 NsUser와 DeleteHistory까지 리팩터링한 부분이 인상적이네요.
QnaDomainService에 대한 본인만의 기준이 있는 것도 좋고요.
2단계 미션이 db 설계 전에 도메인 객체를 먼저 설계하고 구현하는 도전이네요.
이 도전을 통해 기존과는 다른 새로운 경험을 할 수 있기를 기대해 봅니다.
| return this.deleted; | ||
| } | ||
|
|
||
| public void updateDeleted() { |
There was a problem hiding this comment.
| public void updateDeleted() { | |
| protected void updateDeleted() { |
| // TODO : 피드백 주신 부분 잘보았습니다. | ||
| // 저는 개인적으로 도메인 로직은 순수하게 유지하는게 가장 좋다고 생각합니다만. | ||
| // 스프링을 활용한 의존성주입이 필요한 예외적인 상황(불가피한 외부API 호출, 이메일 등등..)에서는 허용하고 사용해주는것도 좋다고 생각합니다. | ||
| // 다만 단계를 거쳐볼것 같아요. 1. 순수 객체 -> 2. 클래스 메서드(static) 집합 객체(=외부 bean 필요없이 캐싱 작업이 필요힌 경우) -> 3. 외부 bean이 불가피하게 필요한경우 bean & di |
2 participants
수강신청 레거시 코드 리팩토링 과제물입니다
실무와 유사한 상황에서 작업하니 전보다 난이도도 높지만 실용적인 재미도도 높네요.
여러가지 부분 가감없이 피드백 부탁드립니다. 아래는 구현하며 고민했던 포인트, 질문사항들 정리 해봤습니다.
질문/고민사항