feat : step 1 #832
feat : step 1 #832
Conversation
javajigi
left a comment
javajigi
left a comment
There was a problem hiding this comment.
1단계 미션 진행 👍
구현한 내용과 관련해 몇 개의 피드백 남겼고요.
리팩터링 과정에 단위 테스트를 추가하면서 점진적으로 리팩터링할 수 있도록 도전해 보면 어떨까요?
| public Answer setDeleted(boolean deleted) { | ||
| this.deleted = deleted; | ||
| return this; | ||
| public void delete() { |
There was a problem hiding this comment.
현재 구현한 방식(삭제 가능 여부와 삭제를 분리함)과 아래와 같이 글쓴이 인지 여부를 이 메서드가 판단하도록 구현하는 것 중 어느 방식으로 구현하는 것이 좋을지 고민해 본다.
위 두 가지 방식 중 선택한 방법으로 구현하는 것이 좋은 이유에 대해 피드백으로 남겨본다.
public void delete(NsUser loginUser){
// 글쓴이 인지를 판단하는 로직 검증 후 삭제 상태로 변경
}| import nextstep.qna.CannotDeleteException; | ||
| import nextstep.users.domain.NsUser; | ||
|
|
||
| public class Answers { |
| } | ||
| } | ||
|
|
||
| public List<DeleteHistory> deleteAllAndCreateHistories(LocalDateTime now) { |
There was a problem hiding this comment.
객체의 삭제 상태를 변경하는 책임과 DeleteHistory 생성하고 반환하는 책임
위와 같이 두 가지 책임을 가지고 있는 느낌이 드는데 어떻게 생각하나?
https://vimeo.com/1137582691 영상에서 추천하는 메서드 이름 짓기 원칙을 따라 이름을 짓고 메서드 분리가 필요하다면 메서드 분리해 보면 어떨까?
이 변경에 따라 QnaServiceTest에 변경이 발생한다면 변경해도 괜찮다.
| + deletedBy + ", createdDate=" + createdDate + "]"; | ||
| } | ||
|
|
||
| public static DeleteHistory forQuestion(Question question, LocalDateTime now) { |
There was a problem hiding this comment.
이와 같이 구현할 경우 삭제 히스토리를 관리해야 하는 도메인 객체가 추가될 때마다 DeleteHistory 모듈이 의존하는 객체의 수가 늘어난다.
객체의 의존 관계는 단방향으로 의존 관계를 맺도록 구현할 것을 추천
| import java.util.ArrayList; | ||
| import java.util.List; | ||
|
|
||
| public class Question { |
There was a problem hiding this comment.
Question 객체는 우리가 현장에서 흔하게 접하는 인스턴스 변수가 많은 객체의 모습이다.
객체 지향 생활 체조 원칙의 다음 두 가지 원칙에 지키기 위해 노력해 본다.
- 규칙 6: 모든 엔티티를 작게 유지한다.
- 규칙 7: 3개 이상의 인스턴스 변수를 가진 클래스를 쓰지 않는다.
특히 규칙 7의 경우 지키기 힘들다 하더라도 인스턴스 변수의 수를 줄이기 위해 도전해 본다.
힌트: 상속을 통한 인스턴스 변수 줄이기, 관련 있는 인스턴스 변수를 새로운 객체로 분리하기 등 활용
javajigi
left a comment
javajigi
left a comment
There was a problem hiding this comment.
ServiceLayer에서 로직이 도메인 객체로 이동하면서 단위 테스트가 가능해졌는데요.
단위 테스트 코드를 추가하지 않았는데 이번 기회에 추가하는 연습을 해보면 어떨까요?
피드백 반영하느라 수고했어요. 👍
| return content.getTitle(); | ||
| } | ||
|
|
||
| public Question setTitle(String title) { |
| public void addAnswer(Answer answer) { | ||
| answer.toQuestion(this); | ||
| answers.add(answer); | ||
| state.addAnswer(answer); |
There was a problem hiding this comment.
답변 목록을 state 객체가 가지는 것이 의미상 잘 연결되지 않는다.
너무 무리한 객체 분리 아닐까?
Answers를 Question의 필드로 가지는 것은 어떨까?
2 participants
No description provided.