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

[렉스] 2단계: 연관 관계 매핑 #43

Open
wants to merge 9 commits into
base: seongwon97
Choose a base branch
from

Conversation

Seongwon97
Copy link

안녕하세요 제이슨
프로젝트를 하다보니 늦게 제출하게 됐습니다ㅠㅠ
죄송합니다

이번 리뷰도 잘 부탁드립니다!!

@Seongwon97 Seongwon97 changed the base branch from main to seongwon97 July 18, 2022 02:03
Copy link

@woowahan-neo woowahan-neo left a comment

Choose a reason for hiding this comment

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

미션을 잘 진행해 주셨습니다 👍
고민해 보면 좋을만한 코멘트 남겨두었으니 확인 부탁드려요~

@@ -7,7 +7,7 @@

public interface AnswerRepository extends JpaRepository<Answer, Long> {

List<Answer> findByQuestionIdAndDeletedFalse(Long questionId);
List<Answer> findByQuestionAndDeletedFalse(Long questionId);

Choose a reason for hiding this comment

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

Suggested change
List<Answer> findByQuestionAndDeletedFalse(Long questionId);
List<Answer> findByQuestionIdAndDeletedFalse(Long questionId);

매개변수와 메서드명이 다르네요.

Copy link
Author

Choose a reason for hiding this comment

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

네오 아무리 수정을 해보려해도 findByQuestionIdAndDeletedFalse라는 이름으로 변경을 하면 오류가 발생하더라고요ㅠ 다른 크루들의 코드와 비교하여도 Answer 도메인의 필드명과 메서드 명이 같은데 오류를 가 발생하는 이유를 잘 모르겠습니다ㅠ

Comment on lines 33 to 45
User savedUser1, savedUser2;
Question savedQuestion1, savedQuestion2;

@BeforeEach
void setUp() {
savedUser1 = userRepository.save(JAVAJIGI);
Question Q1 = new Question("title1", "contents1", savedUser1);
savedQuestion1 = questionRepository.save(Q1);

savedUser2 = userRepository.save(SANJIGI);
Question Q2 = new Question("title2", "contents2", savedUser2);
savedQuestion2 = questionRepository.save(Q2);
}

Choose a reason for hiding this comment

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

테스트 코드도 유지보수 대상입니다.

AnswerRepositoryTest의 인스턴스 변수가 많아지면서 테스트의 가독성이 많이 안좋아졌네요.
해당 값들이 꼭 인스턴스 변수로 필요할지 고민 후 가독성을 높일 수 있는 방법에 대해 고민해보아요.

Copy link
Author

Choose a reason for hiding this comment

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

각각의 테스트 코드 내에서 반복적으로 처리되는 로직때문에 인스턴스 변수와 BeforeEach를 만들었었는데 리펙터링을 통해 인스턴스 변수 제거 및 중복로직 제거를 해봤습니다😁

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants