-
Notifications
You must be signed in to change notification settings - Fork 308
Step1 - 레거시 코드 리팩토링 #809
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Step1 - 레거시 코드 리팩토링 #809
Changes from all commits
46251f3
f71bb0d
6c91f40
0201526
3175b81
0003548
58acd61
6c04e23
ea12de6
c925616
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| package nextstep.common.domain; | ||
|
|
||
| import java.time.LocalDateTime; | ||
| import java.util.Objects; | ||
|
|
||
| public abstract class BaseEntity { | ||
| private Long id; | ||
| private LocalDateTime createdDate; | ||
| private LocalDateTime updatedDate; | ||
|
|
||
| protected BaseEntity(Long id) { | ||
| this(id, LocalDateTime.now(), LocalDateTime.now()); | ||
| } | ||
|
|
||
| public BaseEntity(Long id, LocalDateTime createdDate, LocalDateTime updatedDate) { | ||
| this.id = id; | ||
| this.createdDate = createdDate; | ||
| this.updatedDate = updatedDate; | ||
| } | ||
|
|
||
| public Long getId() { | ||
| return id; | ||
| } | ||
|
|
||
| @Override | ||
| public boolean equals(Object o) { | ||
| if (o == null || getClass() != o.getClass()) { | ||
| return false; | ||
| } | ||
| BaseEntity that = (BaseEntity) o; | ||
| return Objects.equals(id, that.id) | ||
| && Objects.equals(createdDate, that.createdDate) | ||
| && Objects.equals(updatedDate, that.updatedDate); | ||
| } | ||
|
|
||
| @Override | ||
| public int hashCode() { | ||
| return Objects.hash(id, createdDate, updatedDate); | ||
| } | ||
|
|
||
| @Override | ||
| public String toString() { | ||
| return "BaseEntity{" + | ||
| "id=" + id + | ||
| ", createdDate=" + createdDate + | ||
| ", updatedDate=" + updatedDate + | ||
| '}'; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| package nextstep.common.domain; | ||
|
|
||
| import java.time.LocalDateTime; | ||
| import java.util.Objects; | ||
|
|
||
| public abstract class SoftDeletableBaseEntity { | ||
| private Long id; | ||
| private boolean deleted; | ||
| private LocalDateTime createdDate; | ||
| private LocalDateTime updatedDate; | ||
|
|
||
| protected SoftDeletableBaseEntity(Long id) { | ||
| this(id, false, LocalDateTime.now(), LocalDateTime.now()); | ||
| } | ||
|
|
||
| protected SoftDeletableBaseEntity(Long id, LocalDateTime createdAt, LocalDateTime updatedAt) { | ||
| this(id, false, createdAt, updatedAt); | ||
| } | ||
|
|
||
| protected SoftDeletableBaseEntity(Long id, boolean deleted, LocalDateTime createdDate, | ||
| LocalDateTime updatedDate) { | ||
| this.id = id; | ||
| this.deleted = deleted; | ||
| this.createdDate = createdDate; | ||
| this.updatedDate = updatedDate; | ||
| } | ||
|
|
||
| protected boolean isDeleted() { | ||
| return this.deleted; | ||
| } | ||
|
|
||
| public void updateDeleted() { | ||
| this.deleted = true; | ||
| } | ||
|
|
||
| public Long getId() { | ||
| return id; | ||
| } | ||
|
|
||
| @Override | ||
| public boolean equals(Object o) { | ||
| if (o == null || getClass() != o.getClass()) { | ||
| return false; | ||
| } | ||
| SoftDeletableBaseEntity that = (SoftDeletableBaseEntity) o; | ||
| return deleted == that.deleted | ||
| && Objects.equals(id, that.id) | ||
| && Objects.equals(createdDate, that.createdDate) | ||
| && Objects.equals(updatedDate, that.updatedDate); | ||
| } | ||
|
|
||
| @Override | ||
| public int hashCode() { | ||
| return Objects.hash(id, deleted, createdDate, updatedDate); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,71 +1,58 @@ | ||
| package nextstep.qna.domain; | ||
|
|
||
| import nextstep.qna.NotFoundException; | ||
| import nextstep.qna.UnAuthorizedException; | ||
| import nextstep.users.domain.NsUser; | ||
|
|
||
| import java.time.LocalDateTime; | ||
|
|
||
| public class Answer { | ||
| private Long id; | ||
|
|
||
| private NsUser writer; | ||
|
|
||
| import nextstep.common.domain.SoftDeletableBaseEntity; | ||
| import nextstep.qna.exception.unchecked.CannotDeleteException; | ||
| import nextstep.qna.exception.unchecked.NotFoundException; | ||
| import nextstep.qna.exception.unchecked.UnAuthorizedException; | ||
| import nextstep.qna.exception.unchecked.WrongRequestException; | ||
|
|
||
| public class Answer extends SoftDeletableBaseEntity { | ||
| private Long writerId; | ||
| private Question question; | ||
| private BoardContent boardContent; | ||
|
|
||
| private String contents; | ||
|
|
||
| private boolean deleted = false; | ||
|
|
||
| private LocalDateTime createdDate = LocalDateTime.now(); | ||
|
|
||
| private LocalDateTime updatedDate; | ||
|
|
||
| public Answer() { | ||
| public Answer(long writerId, Question question, String contents) { | ||
| this(null, writerId, question, contents); | ||
| } | ||
|
|
||
| public Answer(NsUser writer, Question question, String contents) { | ||
| this(null, writer, question, contents); | ||
| } | ||
|
|
||
| public Answer(Long id, NsUser writer, Question question, String contents) { | ||
| this.id = id; | ||
| if(writer == null) { | ||
| public Answer(Long id, long writerId, Question question, String contents) { | ||
| super(id); | ||
| if (writerId <= 0L) { | ||
| throw new UnAuthorizedException(); | ||
| } | ||
|
|
||
| if(question == null) { | ||
| if (question == null) { | ||
| throw new NotFoundException(); | ||
| } | ||
|
|
||
| this.writer = writer; | ||
| this.writerId = writerId; | ||
| this.question = question; | ||
| this.contents = contents; | ||
| this.boardContent = new BoardContent("" , contents); | ||
| } | ||
|
|
||
| public Long getId() { | ||
| return id; | ||
| public boolean isDeleted() { | ||
| return super.isDeleted(); | ||
| } | ||
|
|
||
| public Answer setDeleted(boolean deleted) { | ||
| this.deleted = deleted; | ||
| return this; | ||
| } | ||
| public void putOnDelete(long requesterId) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
| if (!isOwner(requesterId)) { | ||
| throw new CannotDeleteException("질문을 삭제할 권한이 없습니다."); | ||
| } | ||
|
|
||
| public boolean isDeleted() { | ||
| return deleted; | ||
| super.updateDeleted(); | ||
| } | ||
|
|
||
| public boolean isOwner(NsUser writer) { | ||
| return this.writer.equals(writer); | ||
| public boolean isOwner(long writerId) { | ||
| return this.writerId == writerId; | ||
| } | ||
|
|
||
| public NsUser getWriter() { | ||
| return writer; | ||
| } | ||
| public DeleteHistory createAnswerDeleteHistory(LocalDateTime deletedDateTime) { | ||
| if (!isDeleted()) { | ||
| throw new WrongRequestException("삭제되지 않은 답변은 삭제이력을 생성할 수 없습니다."); | ||
| } | ||
|
|
||
| public String getContents() { | ||
| return contents; | ||
| return new DeleteHistory(ContentType.ANSWER, super.getId(), this.writerId, deletedDateTime); | ||
| } | ||
|
|
||
| public void toQuestion(Question question) { | ||
|
|
@@ -74,6 +61,9 @@ public void toQuestion(Question question) { | |
|
|
||
| @Override | ||
| public String toString() { | ||
| return "Answer [id=" + getId() + ", writer=" + writer + ", contents=" + contents + "]"; | ||
| return "Answer [id=" + super.getId() | ||
| + ", writerId=" + writerId | ||
| + ", contents=" + boardContent | ||
| + "]"; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| package nextstep.qna.domain; | ||
|
|
||
| public class BoardContent { | ||
| private String title; | ||
| private String contents; | ||
|
|
||
| public BoardContent(String title, String contents) { | ||
| this.title = title; | ||
| this.contents = contents; | ||
| } | ||
|
|
||
| @Override | ||
| public String toString() { | ||
| return "BoardContent{" + | ||
| "title='" + title + '\'' + | ||
| ", contents='" + contents + '\'' + | ||
| '}'; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,50 +1,53 @@ | ||
| package nextstep.qna.domain; | ||
|
|
||
| import static java.util.Objects.*; | ||
|
|
||
| import nextstep.common.domain.BaseEntity; | ||
| import nextstep.users.domain.NsUser; | ||
|
|
||
| import java.time.LocalDateTime; | ||
| import java.util.Objects; | ||
|
|
||
| public class DeleteHistory { | ||
| private Long id; | ||
|
|
||
| public class DeleteHistory extends BaseEntity { | ||
| private ContentType contentType; | ||
|
|
||
| private Long contentId; | ||
| private Long deletedId; | ||
|
|
||
| private NsUser deletedBy; | ||
|
|
||
| private LocalDateTime createdDate = LocalDateTime.now(); | ||
|
|
||
| public DeleteHistory() { | ||
| public DeleteHistory(ContentType contentType, Long contentId, Long deletedId, LocalDateTime createdDate) { | ||
| this(0L, contentType, contentId, deletedId, createdDate, null); | ||
| } | ||
|
|
||
| public DeleteHistory(ContentType contentType, Long contentId, NsUser deletedBy, LocalDateTime createdDate) { | ||
| public DeleteHistory(Long id, ContentType contentType, Long contentId, Long deletedId, LocalDateTime createdDate, LocalDateTime updateDate) { | ||
| super(id, createdDate, updateDate); | ||
| this.contentType = contentType; | ||
| this.contentId = contentId; | ||
| this.deletedBy = deletedBy; | ||
| this.createdDate = createdDate; | ||
| this.deletedId = deletedId; | ||
| } | ||
|
|
||
| @Override | ||
| public boolean equals(Object o) { | ||
| if (this == o) return true; | ||
| if (o == null || getClass() != o.getClass()) return false; | ||
| if (o == null || getClass() != o.getClass()) { | ||
| return false; | ||
| } | ||
| DeleteHistory that = (DeleteHistory) o; | ||
| return Objects.equals(id, that.id) && | ||
| contentType == that.contentType && | ||
| Objects.equals(contentId, that.contentId) && | ||
| Objects.equals(deletedBy, that.deletedBy); | ||
| return super.equals(o) | ||
| && contentType == that.contentType | ||
| && Objects.equals(contentId, that.contentId) | ||
| && Objects.equals(deletedId, that.deletedId); | ||
| } | ||
|
|
||
| @Override | ||
| public int hashCode() { | ||
| return Objects.hash(id, contentType, contentId, deletedBy); | ||
| return hash(super.hashCode(), contentType, contentId, deletedId); | ||
| } | ||
|
|
||
| @Override | ||
| public String toString() { | ||
| return "DeleteHistory [id=" + id + ", contentType=" + contentType + ", contentId=" + contentId + ", deletedBy=" | ||
| + deletedBy + ", createdDate=" + createdDate + "]"; | ||
| return "DeleteHistory{" + | ||
| "id=" + super.toString() + | ||
| ", contentType=" + contentType + | ||
| ", contentId=" + contentId + | ||
| ", deletedId=" + deletedId + | ||
| '}'; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| package nextstep.qna.domain; | ||
|
|
||
| import java.time.LocalDateTime; | ||
| import java.util.List; | ||
|
|
||
| public class QnADomainService { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 새로운 DomainService를 분리한 점이 인상적임 👍 |
||
|
|
||
| // TODO : 피드백 주신 부분 잘보았습니다. | ||
| // 저는 개인적으로 도메인 로직은 순수하게 유지하는게 가장 좋다고 생각합니다만. | ||
| // 스프링을 활용한 의존성주입이 필요한 예외적인 상황(불가피한 외부API 호출, 이메일 등등..)에서는 허용하고 사용해주는것도 좋다고 생각합니다. | ||
| // 다만 단계를 거쳐볼것 같아요. 1. 순수 객체 -> 2. 클래스 메서드(static) 집합 객체(=외부 bean 필요없이 캐싱 작업이 필요힌 경우) -> 3. 외부 bean이 불가피하게 필요한경우 bean & di | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
| public List<DeleteHistory> deleteQuestion( | ||
| long requesterId, Question question, LocalDateTime fixedDeletedDateTime | ||
| ) { | ||
| question.putOnDelete(requesterId); | ||
|
|
||
| return question.bringAllDeleteHistories(fixedDeletedDateTime); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.