[Step2] 연관 관계 매핑 #58
[Step2] 연관 관계 매핑 #58
Conversation
src/main/java/qna/domain/Answer.java
Outdated
| @JoinColumn(name = "writer_id", | ||
| referencedColumnName = "ID", | ||
| foreignKey = @ForeignKey(name = "fk_answer_writer")) | ||
| @ManyToOne | ||
| private User writer; |
There was a problem hiding this comment.
연관관계 설정 잘해주셨네요 👍
기본 PK값으로 될텐데 referencedColumnName을 따로 설정하신 이유가 있으신가요?
There was a problem hiding this comment.
조금더 명시적이면 좋겠다라고 판단해서 작성했습니다만, 혹시 잘못된걸까요!?
There was a problem hiding this comment.
referencedColumnName 이 정의되어 있어 혹시 다른 컬럼을 지정하셨나 했습니다.
설정해보는 것도 좋다고 생각합니다 😄
그리고 보통 아래와 같은 형태로 작성을 합니다.
@ManyToOne
@JoinColumn(name = ... )
There was a problem hiding this comment.
답글 주셨군요 ㅎㅎ 감사합니다. referencedColumnName 이라는게 있어서 사용해본 거라 제거하도록 하겠습니다 : )
src/main/java/qna/domain/Answer.java
Outdated
|
|
||
| import qna.NotFoundException; | ||
| import qna.UnAuthorizedException; | ||
|
|
||
| @Entity | ||
| public class Answer extends BaseEntity { | ||
|
|
||
| public static final Answer NONE = new Answer(); |
There was a problem hiding this comment.
테스트를 위한 코드를 도메인에 추가하는 것은 되도록 지양하는게 좋겠네요.
아래 피드백을 참고해주세요! #58 (comment)
src/main/java/qna/domain/Answer.java
Outdated
| public void setQuestion(Question question) { | ||
| this.question = question; | ||
| } |
There was a problem hiding this comment.
객체의 상태 값을 외부에서 변경할 수 있도록 열어두기 때문에 불필요한 setter()는 정리하는게 좋겠네요.
setDeleted()는 delete() 라는 네이밍으로 변경해보면 어떨까요?
There was a problem hiding this comment.
맞습니다..! 불필요한 메소드는 노출하지 않은게 정답이라고 생각합니다. 코드 수정하겠습니다 ㅎㅎ
|
|
||
| Answer save = repository.save(answer); | ||
|
|
||
| Answer answer1 = repository.findByIdAndDeletedFalse(save.getId()).orElse(Answer.NONE); |
There was a problem hiding this comment.
빈 객체를 반환하기 보다는 예외를 던져 처리해보면 좋겠네요 :)
| Answer answer1 = repository.findByIdAndDeletedFalse(save.getId()).orElse(Answer.NONE); | |
| Answer answer1 = repository.findByIdAndDeletedFalse(save.getId()).orElseThrow(EntityNotFoundException::new); |
| Answer answer2 = repository.findByIdAndDeletedFalse(save.getId()).orElse(Answer.NONE); | ||
| assertThat(answer2).isEqualTo(Answer.NONE); |
There was a problem hiding this comment.
아래처럼 테스트 해보면 어떨까요?
assertThatThrownBy(() ->
repository.findByIdAndDeletedFalse(save.getId())
.orElseThrow(EntityNotFoundException::new)
).isInstanceOf(EntityNotFoundException.class);There was a problem hiding this comment.
엇! 테스트를 위한 코드는 조심해야된다는 말 명심하겠습니다 ㅎㅎ
| User user = userRepository.save(UserTest.JAVAJIGI); | ||
| Question q1 = QuestionTest.Q1.writeBy(user); | ||
| Question q2 = QuestionTest.Q2.writeBy(user); |
There was a problem hiding this comment.
테스트시 다른 테스트에 영향을 받지 않고 독립된 테스트가 실행되어야 하는데요.
static 변수를 테스트 내에서 사용하는 것에 대해 고민해보시면 좋을것 같아요.
변경을 위한 피드백은 아니니 미션을 진행하시면서 문제 발생시 고려해보시면 좋겠네요 :)
There was a problem hiding this comment.
맞습니다. 이 부분때문에, 처음에 한 개 테스트를 실행 시킬때는 문제가 없었으나, 다중 테스트를 실행시키는 과정에서 문제가 발생하더라구요.
이 부분은 지양하도록 하겠습니다.
- 테스트를 위한 테스트 코드 NONE 제거 - NONE 제거로 인한 테스트 코드 변경 - 불필요하게 노출된 setter 제거
| @ManyToOne | ||
| @JoinColumn(name = "writer_id", | ||
| foreignKey = @ForeignKey(name = "fk_answer_writer")) | ||
| private User writer; | ||
|
|
||
| @ManyToOne | ||
| @JoinColumn(name = "question_id", | ||
| foreignKey = @ForeignKey(name = "fk_answer_to_question")) | ||
| private Question question; |
There was a problem hiding this comment.
Answer를 조회시 항상 Question, Writer를 같이 조회하는게 아니라면 지연로딩 속성을 주면 좋겠네요.
@OneToMany는 LAZY가 디폴트이지만 @ ManyToOne는 아니기 때문입니다.
@ManyToOne(fetch = FetchType.LAZY)
안녕하세요 ㅎㅎ 리뷰어님.
JPA을 학습하면서 하려니, 생각만큼 개발의 속도가 붙지 않네요.
보내주신 피드백 감사합니다 : )
이번 코드의 피드백도 잘 부탁드립니다.
이번에 개발하면서 일부 어려웠던 부분은
테스트 코드 안에서 하나의 EntityPersistance 영역을 공유하다보니, 단위테스트는 통과하는데, 전체 테스트할때 Fail 나는 경우가 더러 발생해- 조금 고생했네요 ㅠ
리뷰 잘 부탁드립니다!