-
Notifications
You must be signed in to change notification settings - Fork 0
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
[REFACTOR/#169] 성지순례 카테고리 조회 리팩토링 및 서비스 테스트 추가 #175
Conversation
추가 사항 - Nested class로 분리 - 로그인 상태에서 성지순례 애니별 카테고리 조회 성공 테스트 추가
변경 내역 - Category 관련 조회 서비스 분리
변경 내역 - 인터페이스 메서드 간략하게 수정 - 메서드 분리
변경 내역 - 기존 하드코딩에서 한 눈에 추가되는 값을 확인할 수 있도록 수정
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.
수고하셨어요!!! Service 분리 한 거 좋은 시도인 거 같아요오
테스트코드 열심히 더 공부해올게여 .. 총총👣
|
||
// Lombok | ||
compileOnly 'org.projectlombok:lombok' |
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.
요거 implementation
으로 바꾸신 이유가 있나요?? (단순 궁금증)
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.
테스트코드 작성할 때 롬복이 필요해서 수정했어요!
찾아보니까 compileOnly + testCompileOnly 쓰는것보다 implementation만 쓰는게 더 무겁다고 하네요...
gradle 파일 길어지면 관리가 까다로워질까봐 수정한건데 원래대로 되돌리는게 더 좋을거 같아요!
+) 수정했습니다 :)
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.
오..새로운 사실 덕분에 알아가요 좋습니다!!
@@ -79,24 +77,26 @@ public RallyResponseDto.RallyDetailResponseDto getRallyDetail(Long rallyId, Memb | |||
*/ | |||
public RallyResponseDto.RallyAddressListDto getRallyAddressList(Long rallyId, Member member) { | |||
Rally rally = rallyRepository.findById(rallyId).orElseThrow( | |||
()-> new RestApiException(ErrorCode.RALLY_NOT_FOUND)); | |||
() -> new RestApiException(ErrorCode.RALLY_NOT_FOUND)); | |||
|
|||
// 주소, 성지순례 리스트 정보 담은 주소 리스트 생성 | |||
List<Address> addressList = addressRepository.findByPilgrimages_Rally(rally); | |||
System.out.println(addressList.size()); |
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.
요건 없어도 되는 줄 맞나요?!
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.
헉 맞습니다 지울게요!
private PilgrimageCategoryQueryService pilgrimageService; | ||
|
||
@BeforeEach | ||
void setup() { | ||
this.pilgrimageService = new PilgrimageCategoryQueryService(memberRepository, rallyRepository, | ||
visitedPilgrimageRepository, addressRepository, pilgrimageRepository); | ||
} |
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.
@InjectMocks
를 붙이는 대신에 setUp()
메서드로 @Mock
으로 만든 객체들을 주입한 후 pilgrimageService
인스턴스를 생성하신 이유가 궁금해요!
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.
injectMocks 사용이 낯설어서 직접 주입하는 방식을 선택했습니다
덕분에 편리한 방법을 알았어요 감사합니다!
📄 Work Description
변경 사항
PilgrimageCategoryQueryService
/PilgrimageQueryService
분리⚙️ ISSUE
📷 Screenshot
public 메서드 가독성 위주로 리팩토링 진행했습니다. ex)
PilgrimageCategoryQueryService 리팩토링 진행 전
PilgrimageCategoryQueryService 리팩토링 진행 후
💬 To Reviewers
Mockito 사용해서 의존성 최대한 배제하고 테스트 코드를 작성했으나,
모킹 놓친 부분이 있을 수 있습니다. 혹시 발견하신다면 피드백 부탁드립니다.
그 외 모든 사항 피드백 환영합니당
🔗 Reference
문제를 해결하면서 도움이 되었거나, 참고했던 사이트(코드링크)
https://bcp0109.tistory.com/317