Skip to content
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

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

Open
wants to merge 1 commit into
base: kmson92
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
feat: 리팩토링
kmson92 committed Dec 14, 2023
commit 9fbd55477a0f903d349e5c5bfae42bf8f27f5272
15 changes: 15 additions & 0 deletions src/main/java/nextstep/qna/domain/Answer.java
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
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;
import java.util.List;

public class Answer {
private Long id;
@@ -72,8 +74,21 @@ public void toQuestion(Question question) {
this.question = question;
}

public void delete(NsUser loginUser) throws CannotDeleteException {
if (!isOwner(loginUser)) {
throw new CannotDeleteException("다른 사람이 쓴 답변이 있어 삭제할 수 없습니다.");
}
deleted = true;
}

public DeleteHistory deleteHistory() {
return new DeleteHistory(ContentType.ANSWER, id, writer, LocalDateTime.now());
}

@Override
public String toString() {
return "Answer [id=" + getId() + ", writer=" + writer + ", contents=" + contents + "]";
}


}
33 changes: 33 additions & 0 deletions src/main/java/nextstep/qna/domain/Answers.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package nextstep.qna.domain;

import nextstep.qna.CannotDeleteException;
import nextstep.users.domain.NsUser;

import java.util.ArrayList;
import java.util.List;

public class Answers {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Answers에 대한 단위 테스트도 작성해보면 어떨까요?

private List<Answer> answers;

public Answers(List<Answer> answers) {
this.answers = answers;
}

public void add(Answer answer) {
answers.add(answer);
}

public void delete(NsUser loginUser) throws CannotDeleteException {
for(Answer answer : answers) {
answer.delete(loginUser);
}
}

public List<DeleteHistory> deleteHistories() {
Comment on lines +20 to +26

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

answer.deleteHistory 내부에서 LocalDateTime.now()를 사용하고 있는 것 같아요.

delete 시점과 deleteHistories가 호출된 시점이 약간 차이가 있을 수 있지 않을까요?

List<DeleteHistory> deleteHistories = new ArrayList<>();
for(Answer answer : answers) {
deleteHistories.add(answer.deleteHistory());
}
return deleteHistories;
}
}
22 changes: 20 additions & 2 deletions src/main/java/nextstep/qna/domain/Question.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package nextstep.qna.domain;

import nextstep.qna.CannotDeleteException;
import nextstep.users.domain.NsUser;

import java.time.LocalDateTime;
@@ -15,7 +16,7 @@ public class Question {

private NsUser writer;

private List<Answer> answers = new ArrayList<>();
private Answers answers = new Answers(new ArrayList<>());

private boolean deleted = false;

@@ -81,12 +82,29 @@ public boolean isDeleted() {
return deleted;
}

public List<Answer> getAnswers() {
public Answers getAnswers() {
return answers;
}

@Override
public String toString() {
return "Question [id=" + getId() + ", title=" + title + ", contents=" + contents + ", writer=" + writer + "]";
}

public void delete(NsUser loginUser) throws CannotDeleteException {
if (!isOwner(loginUser)) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isOwner의 내부를 보면 아래와 같은 메서드를 호출하고 있습니다.

writer.equals(loginUser);

equals가 적절히 구현되어있는지도 한번 확인해보시면 좋을 것 같네요!

throw new CannotDeleteException("질문을 삭제할 권한이 없습니다.");
}
this.deleted = true;
answers.delete(loginUser);
}

public List<DeleteHistory> deleteHistories() {
List<DeleteHistory> deleteHistories = new ArrayList<>();
deleteHistories.add(new DeleteHistory(ContentType.QUESTION, id, writer, LocalDateTime.now()));
deleteHistories.addAll(answers.deleteHistories());

return deleteHistories;
}

}
20 changes: 2 additions & 18 deletions src/main/java/nextstep/qna/service/QnAService.java
Original file line number Diff line number Diff line change
@@ -26,24 +26,8 @@ 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("질문을 삭제할 권한이 없습니다.");
}

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()));
}
question.delete(loginUser);
List<DeleteHistory> deleteHistories = question.deleteHistories();
deleteHistoryService.saveAll(deleteHistories);
}
}
20 changes: 20 additions & 0 deletions src/test/java/nextstep/qna/domain/AnswerTest.java
Original file line number Diff line number Diff line change
@@ -1,8 +1,28 @@
package nextstep.qna.domain;

import nextstep.qna.CannotDeleteException;
import nextstep.users.domain.NsUserTest;
import org.junit.jupiter.api.Test;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;

public class AnswerTest {
public static final Answer A1 = new Answer(NsUserTest.JAVAJIGI, QuestionTest.Q1, "Answers Contents1");
public static final Answer A2 = new Answer(NsUserTest.SANJIGI, QuestionTest.Q1, "Answers Contents2");

@Test
void 글쓴이_로그인유저_같을때() throws CannotDeleteException {
A1.delete(NsUserTest.JAVAJIGI);
assertThat(A1.isDeleted()).isTrue();
}

@Test
void 글쓴이_로그인유저_다를때() {
assertThatThrownBy(() -> {
A1.delete(NsUserTest.SANJIGI);
}).isInstanceOf(CannotDeleteException.class)
.hasMessage("다른 사람이 쓴 답변이 있어 삭제할 수 없습니다.");
}
}

19 changes: 19 additions & 0 deletions src/test/java/nextstep/qna/domain/QuestionTest.java
Original file line number Diff line number Diff line change
@@ -1,8 +1,27 @@
package nextstep.qna.domain;

import nextstep.qna.CannotDeleteException;
import nextstep.users.domain.NsUserTest;
import org.junit.jupiter.api.Test;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;

public class QuestionTest {
public static final Question Q1 = new Question(NsUserTest.JAVAJIGI, "title1", "contents1");
public static final Question Q2 = new Question(NsUserTest.SANJIGI, "title2", "contents2");

@Test
void 글쓴이_로그인유저_같을때() throws CannotDeleteException {
Q1.delete(NsUserTest.JAVAJIGI);
assertThat(Q1.isDeleted()).isTrue();
}

@Test
void 글쓴이_로그인유저_다를때() {
assertThatThrownBy(() -> {
Q1.delete(NsUserTest.SANJIGI);
}).isInstanceOf(CannotDeleteException.class)
.hasMessage("질문을 삭제할 권한이 없습니다.");
}
}