[YS-254] refactor: CustomFilter 도메인 모델에 팩토리 메서드 추가 및 Pagination 클래스 이동 #78
Conversation
|
Warning Rate limit exceeded@Ji-soo708 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 1 minutes and 28 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (10)
Walkthrough이 PR은 실험 관련 입력 데이터를 도메인 모델로 변환하는 매핑 로직을 개선합니다. Changes
Sequence Diagram(s)sequenceDiagram
participant 클라 as 클라이언트
participant UC as GetExperimentPostsUseCase
participant DM as 도메인모델 (CustomFilter, Pagination)
participant GW as ExperimentPostGateway
클라->>UC: 요청 입력 전달
UC->>DM: CustomFilter.newCustomFilter() 호출
UC->>DM: Pagination.newPagination() 호출
UC->>GW: findExperimentPostsByCustomFilter(필터, 페이징) 호출
GW-->>UC: 결과 반환
UC-->>클라: 응답 반환
sequenceDiagram
participant 클라 as 클라이언트
participant UC as GetExperimentPostTotalCountByCustomFilterUseCase
participant DM as 도메인모델 (CustomFilter)
participant DL as 데이터 계층
클라->>UC: 필터 입력 전달
UC->>DM: CustomFilter.newCustomFilter() 호출
UC->>DL: 총 수 조회 요청 (필터 전달)
DL-->>UC: 총 수 반환
UC-->>클라: 결과 반환
Possibly related PRs
Suggested labels
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/main/kotlin/com/dobby/backend/domain/model/experiment/Pagination.kt (1)
8-10: 입력값 유효성 검사 로직 추가를 제안드립니다.페이지네이션에 사용되는
page와count값에 대한 유효성 검사가 필요해 보입니다.다음과 같이 팩토리 메서드에 유효성 검사를 추가하는 것을 고려해보세요:
fun newPagination(page: Int, count: Int): Pagination { + require(page >= 0) { "페이지 번호는 0 이상이어야 합니다." } + require(count > 0) { "페이지당 항목 수는 0보다 커야 합니다." } return Pagination(page, count) }src/main/kotlin/com/dobby/backend/application/usecase/experiment/GetExperimentPostTotalCountByCustomFilterUseCase.kt (1)
24-29: 팩토리 메서드 패턴이 잘 적용되었습니다.매퍼 클래스를 제거하고 도메인 모델의 팩토리 메서드를 사용하도록 리팩토링한 부분이 다음과 같은 장점이 있습니다:
- 객체 생성 로직이 도메인 모델에 캡슐화되어 응집도가 향상됨
- null 처리를 위한 안전한 호출 연산자 사용이 적절함
하나의 제안사항으로는, 팩토리 메서드에 전달되는 파라미터들의 가독성을 높이기 위해 named arguments를 사용하는 것을 고려해보시면 좋을 것 같습니다:
CustomFilter.newCustomFilter( - input.matchType, - studyTarget = input.studyTarget?.let { StudyTarget(it.gender, it.age) }, - locationTarget = input.locationTarget?.let { LocationTarget(it.region, it.areas) }, - input.recruitStatus + matchType = input.matchType, + studyTarget = input.studyTarget?.let { StudyTarget(it.gender, it.age) }, + locationTarget = input.locationTarget?.let { LocationTarget(it.region, it.areas) }, + recruitStatus = input.recruitStatus )src/main/kotlin/com/dobby/backend/domain/model/experiment/CustomFilter.kt (1)
14-23: 팩토리 메서드의 활용도 개선이 필요합니다.현재 팩토리 메서드는 단순히 생성자에 파라미터를 전달하는 역할만 수행하고 있습니다. 다음과 같은 개선사항을 고려해보시기 바랍니다:
- 기본값을 제공하는 오버로드된 메서드 추가
- 유효성 검사 로직 추가
- 비즈니스 규칙에 따른 객체 생성 제약 추가
예시 코드:
companion object { fun newCustomFilter( matchType: MatchType?, studyTarget: StudyTarget?, locationTarget: LocationTarget?, recruitStatus: RecruitStatus - ): CustomFilter = CustomFilter(matchType, studyTarget, locationTarget, recruitStatus) + ): CustomFilter { + // 유효성 검사 예시 + require(!(matchType == null && studyTarget == null && locationTarget == null)) { + "적어도 하나의 필터 조건이 필요합니다" + } + + return CustomFilter(matchType, studyTarget, locationTarget, recruitStatus) + } + + // 기본값을 제공하는 오버로드된 메서드 예시 + fun newCustomFilter( + matchType: MatchType? + ): CustomFilter = newCustomFilter( + matchType = matchType, + studyTarget = null, + locationTarget = null, + recruitStatus = RecruitStatus.ALL + ) }src/main/kotlin/com/dobby/backend/application/usecase/experiment/GetMyExperimentPostsUseCase.kt (1)
35-35: 페이지네이션 처리를 더욱 간결하게 개선할 수 있습니다.현재 구현도 좋지만, Kotlin의 구조 분해 선언을 활용하면 코드를 더욱 간결하게 만들 수 있습니다.
다음과 같이 개선해 보세요:
- pagination = Pagination.newPagination(input.pagination.page, input.pagination.count), + pagination = input.pagination.let { (page, count) -> Pagination.newPagination(page, count) },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/main/kotlin/com/dobby/backend/application/mapper/ExperimentMapper.kt(0 hunks)src/main/kotlin/com/dobby/backend/application/mapper/MemberMapper.kt(0 hunks)src/main/kotlin/com/dobby/backend/application/usecase/experiment/GetExperimentPostTotalCountByCustomFilterUseCase.kt(2 hunks)src/main/kotlin/com/dobby/backend/application/usecase/experiment/GetExperimentPostsUseCase.kt(2 hunks)src/main/kotlin/com/dobby/backend/application/usecase/experiment/GetMyExperimentPostsUseCase.kt(2 hunks)src/main/kotlin/com/dobby/backend/domain/model/experiment/CustomFilter.kt(1 hunks)src/main/kotlin/com/dobby/backend/domain/model/experiment/Pagination.kt(1 hunks)src/test/kotlin/com/dobby/backend/application/usecase/experiment/GetExperimentPostsUseCaseTest.kt(6 hunks)
💤 Files with no reviewable changes (2)
- src/main/kotlin/com/dobby/backend/application/mapper/MemberMapper.kt
- src/main/kotlin/com/dobby/backend/application/mapper/ExperimentMapper.kt
🧰 Additional context used
📓 Learnings (1)
src/main/kotlin/com/dobby/backend/application/usecase/experiment/GetMyExperimentPostsUseCase.kt (2)
Learnt from: Ji-soo708
PR: YAPP-Github/25th-Web-Team-2-BE#77
File: src/main/kotlin/com/dobby/backend/application/usecase/member/email/GetMatchingExperimentPostsUseCase.kt:12-14
Timestamp: 2025-02-03T01:27:29.754Z
Learning: The `requestTime` parameter in `GetMatchingExperimentPostsUseCase` is used in conjunction with `lastProcessedTime` in `ExperimentPostCustomRepositoryImpl` to filter experiment posts created between the last processed time and the current time, preventing duplicate processing of the same posts.
Learnt from: Ji-soo708
PR: YAPP-Github/25th-Web-Team-2-BE#77
File: src/main/kotlin/com/dobby/backend/application/usecase/member/email/GetMatchingExperimentPostsUseCase.kt:16-18
Timestamp: 2025-02-03T01:27:08.183Z
Learning: In Kotlin, prefer using non-nullable collections (e.g., `List<T>`) with `emptyList()` as default value instead of nullable collections (e.g., `List<T>?`) to improve null safety and reduce the need for null checks.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (15)
src/main/kotlin/com/dobby/backend/domain/model/experiment/Pagination.kt (1)
3-12: 팩토리 메서드 패턴이 잘 적용되었습니다!도메인 모델에 팩토리 메서드를 추가하여 객체 생성 로직을 캡슐화한 것이 좋습니다. 매퍼 클래스를 제거하고 도메인 모델에서 직접 인스턴스를 생성하는 방식으로 리팩토링한 것이 코드의 응집도를 높였습니다.
src/main/kotlin/com/dobby/backend/application/usecase/experiment/GetExperimentPostTotalCountByCustomFilterUseCase.kt (1)
5-7: 도메인 모델 임포트 추가가 적절합니다.매퍼 클래스 제거에 따른 도메인 모델 직접 사용을 위해 필요한 임포트들이 잘 추가되었습니다.
src/main/kotlin/com/dobby/backend/domain/model/experiment/CustomFilter.kt (1)
25-33: LGTM! StudyTarget과 LocationTarget 클래스의 구조가 명확합니다.데이터 클래스들이 잘 구조화되어 있으며, nullable 타입의 사용이 적절합니다.
src/main/kotlin/com/dobby/backend/application/usecase/experiment/GetMyExperimentPostsUseCase.kt (2)
5-5: 도메인 모델 직접 사용으로의 전환을 승인합니다.매퍼 클래스 제거 및 도메인 모델 직접 사용이라는 리팩토링 목표에 부합하는 변경사항입니다.
39-48: 널 안전성 처리가 잘 구현되어 있습니다.
?.연산자와emptyList()를 사용한 널 처리 방식이 Kotlin의 베스트 프랙티스를 잘 따르고 있습니다.src/main/kotlin/com/dobby/backend/application/usecase/experiment/GetExperimentPostsUseCase.kt (4)
5-8: 도메인 모델 직접 참조로 단순화
새로운 도메인 모델 클래스를 직접 임포트하여 사용하므로, 기존 매퍼 의존성을 제거하고 코드 가독성을 높였습니다. 변경된 임포트는 문제 없어 보입니다.
65-70: Factory 메서드 활용으로 로직 명확화
CustomFilter.newCustomFilter를 사용하여 직관적으로 필터 객체를 생성하였습니다. 파라미터도 적절하게 매핑되고 있어 문제가 없어 보입니다.
71-74: Pagination 객체 생성 논리 일관성 유지
Pagination.newPagination으로 페이징 설정을 명확히 드러내어, 코드를 이해하기가 더욱 쉬워졌습니다.
76-77: 도메인 객체 전달로 가독성 향상
domainFilter와pagination을 명시적으로 생성 후 게이트웨이에 전달하여, 의도가 분명하고 유지보수가 용이해졌습니다.src/test/kotlin/com/dobby/backend/application/usecase/experiment/GetExperimentPostsUseCaseTest.kt (6)
9-9: 도메인 모델 임포트 간소화
com.dobby.backend.domain.model.experiment.*임포트를 사용해 테스트 전체에서 도메인 클래스를 일괄적으로 관리하므로, 유지보수에 편리해 보입니다.
85-92: 테스트에서 Factory 메서드 직접 사용
CustomFilter.newCustomFilter와Pagination.newPagination호출로, 매핑 로직 없이 도메인 객체를 직접 생성해 테스트를 명확히 표현하고 있습니다.
165-173: Null 시나리오 처리를 잘 반영
studyTarget이 null인 경우에도let을 통해 안전하게 객체 생성을 처리하고 있어서, 테스트가 도메인 로직을 정확히 검증합니다.
245-252: locationTarget의 areas null 처리
locationTarget의areas가 null이어도 일관된 방식으로 테스트를 진행하는 모습이 돋보입니다. 코드 품질이 향상되었습니다.
324-330: 필터 결과가 없는 경우 테스트
빈 결과를 반환하는 시나리오에 대한 테스트 케이스가 명확합니다. 도메인 객체 생성 로직과 함께 잘 맞물려 동작하고 있습니다.
402-408: 필터가 없을 때 전체 공고 반환 테스트
필터가 없어도CustomFilter.newCustomFilter를 통한 객체 생성이 가능하고, 올바른 결과(전체 공고)를 반환하는지 확인하는 테스트가 적절합니다.
There was a problem hiding this comment.
이번 회의 때 해당 리팩터링에 대해 먼저 논의하고 진행하려 했으나, 회의 전에 시간이 남아 미리 작업을 진행하고 수정님의 의견을 듣고자 PR을 먼저 올렸습니다. 👋
해당 PR에서는 아래와 같은 이유로 기존의 mapper 클래스를 제거하고, 도메인 모델 내의 팩토리 메서드를 활용하여 객체를 생성하도록 리팩터링하였습니다.
-
일관성 있는 객체 생성
기존에Member,ExperimentPost등 도메인 모델들은 이미 팩토리 메서드를 통해 객체를 생성하고 있었습니다.Pagination이나CustomFilter역시 도메인 모델에 포함되어 있으므로, 별도의 mapper 클래스를 두지 않고 도메인 모델의 팩토리 메서드를 활용하면 전체 코드의 통일성과 일관성을 높일 수 있을 거라 기대합니다! -
재사용성 향상
mapper 클래스를 사용하면 공통된 객체에 매핑할 때도 각 유즈케이스마다 별도로 매퍼를 작성해야 하는 단점이 있었습니다. 도메인 모델 내부에 객체 생성 로직을 캡슐화함으로써, 여러 유즈케이스에서 동일한 객체 생성 방식을 재사용할 수 있게 되어 중복 코드가 줄이고자 했습니다. -
클린 아키텍처 원칙 준수
도메인 모델이 스스로 자신의 상태를 관리하고 객체 생성을 담당하게 함으로써, 애플리케이션 계층과 인프라 계층의 의존성을 최소화하고 도메인 계층의 순수성을 유지하고자 했습니다.
| given("studyTarget이 null인 경우") { | ||
| val customFilter = CustomFilterInput( | ||
| matchType = MatchType.ALL, | ||
| studyTarget = null, | ||
| locationTarget = LocationTargetInput(region = Region.SEOUL, areas = listOf(Area.SEODAEMUNGU)), |
|
CustomFilter나 Pagination이 이미 도메인 모델로 정의되어 있어서, 재사용성을 높이려는 리팩터링이 인상 깊었어요. '도메인 모델은 비즈니스 로직을 다룬다'라는 클린 아키텍처의 원칙을 일전의 회의 이후로 다시 곰곰이 생각해봤는데요. 이미 도메인 모델에서 Pagination이 정의되어 있어서 이를 활용하려는 시도는 재사용성의 측면에서 타당하다고 생각이 들지만, 이 부분에 대해 어떻게 생각하시나요? 지수님의 의견이 듣고 싶습니다. |
@chock-cho 오 좋은 의견입니다. 수정님의 의견을 듣고 보니, 단순히 page와 count를 담는 꼼꼼하게 봐주셔서 감사합니다. |
| @@ -30,8 +29,7 @@ import org.springframework.web.bind.annotation.* | |||
| @RestController | |||
| @RequestMapping("/v1/experiment-posts") | |||
| class ExperimentPostController ( | |||
| private val experimentPostService: ExperimentPostService, | |||
| private val paginationService: PaginationService | |||
There was a problem hiding this comment.
수정님의 코멘트를 보고 PaginationService는 단순히 isLastPage()라는 비교 연산만 수행하는데, 이건 비즈니스 로직이라기보다는 API 응답을 위한 보조 계산이라 생각이 들었어요.
그래서 불필요한 서비스 계층을 유지하기보다는, API 컨트롤러에서 바로 처리하는 게 더 간결하고 유지보수하기 좋은 구조라 생각이 들어 삭제했습니다!
Ji-soo708
changed the title
#78) * feat: add factory method to domain model class * refactor: use factory method instead of mapper in usecase * refactor: delete unused mapper class * refactor: use factory method for Pagination creation with let destructuring * refactor: move Pagination from domain to application layer
#78) * feat: add factory method to domain model class * refactor: use factory method instead of mapper in usecase * refactor: delete unused mapper class * refactor: use factory method for Pagination creation with let destructuring * refactor: move Pagination from domain to application layer
💡 작업 내용
✅ 셀프 체크리스트
🙋🏻 확인해주세요
🔗 Jira 티켓
https://yappsocks.atlassian.net/browse/YS-254
Summary by CodeRabbit