refactor(user): Add user domain convert oas model #41
Conversation
Walkthrough이 변경 사항은 사용자 관련 모델 변환을 위한 새로운 Changes
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 (
|
|
어떤 식으로 할지 고민되어서 우선 function 으로 converting 관리하는 방법으로 해봤어요 |
![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 (3)
bootstrap/api/src/main/kotlin/com/threedays/bootstrap/api/user/OASModelAdapter.kt (3)
24-44: 복잡한 변환 로직 분리 제안현재 구현은 정확하지만, 변환 로직이 다소 복잡합니다. 가독성과 유지보수성을 높이기 위해 중첩된 객체 변환을 별도의 확장 함수로 분리하는 것이 좋습니다.
다음과 같은 리팩토링을 제안합니다:
+private fun com.threedays.domain.user.entity.JobOccupation.toDisplayInfo() = + JobOccupationDisplayInfo( + code = JobOccupation.valueOf(this.name), + display = this.koreanName + ) +private fun com.threedays.domain.user.entity.Location.toDisplayInfo() = + LocationDisplayInfo( + id = this.id.value, + display = this.display, + ) fun com.threedays.domain.user.entity.UserProfile.toUserProfileDisplayInfo(): UserProfileDisplayInfo = UserProfileDisplayInfo( gender = Gender.valueOf(this.gender.name), birthYear = this.birthYear.value, - jobOccupation = JobOccupationDisplayInfo( - code = JobOccupation.valueOf(this.jobOccupation.name), - display = this.jobOccupation.koreanName - ), + jobOccupation = this.jobOccupation.toDisplayInfo(), - locations = this.locations.map { - LocationDisplayInfo( - id = it.id.value, - display = it.display, - ) - }, + locations = this.locations.map { it.toDisplayInfo() }, company = this.company?.let { CompanyDisplayInfo( id = it.id.value, display = it.display, ) }, )
54-62: 양방향 변환에 대한 테스트 케이스 필요도메인 모델과 OAS 모델 간의 양방향 변환이 구현되어 있습니다. 이러한 양방향 변환의 정확성을 보장하기 위해 변환 후 원래 상태로 복원되는지 확인하는 테스트가 필요합니다.
양방향 변환 테스트 코드 작성을 도와드릴까요?
64-75: 변환 로직 단순화 제안현재 구현이 올바르게 작동하지만, 더 간결하게 작성할 수 있습니다.
다음과 같은 리팩토링을 제안합니다:
fun com.threedays.domain.user.entity.UserDesiredPartner.toOASModel() = UserDesiredPartner( - jobOccupations = this.jobOccupations.map { - JobOccupation.valueOf(it.name) - }, + jobOccupations = this.jobOccupations.map { JobOccupation.valueOf(it.name) }, birthYearRange = this.birthYearRange.let { BirthYearRange( start = it.start?.value, end = it.end?.value, ) }, preferDistance = PreferDistance.valueOf(this.preferDistance.name), )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
bootstrap/api/src/main/kotlin/com/threedays/bootstrap/api/user/OASModelAdapter.kt(1 hunks)bootstrap/api/src/main/kotlin/com/threedays/bootstrap/api/user/UserController.kt(5 hunks)
🧰 Additional context used
📓 Learnings (1)
bootstrap/api/src/main/kotlin/com/threedays/bootstrap/api/user/UserController.kt (1)
Learnt from: waterfogSW
PR: Student-Center/3days-server#22
File: bootstrap/api/src/main/kotlin/com/threedays/bootstrap/api/user/UserController.kt:23-24
Timestamp: 2024-11-12T13:22:13.201Z
Learning: `UserController` (`bootstrap/api/src/main/kotlin/com/threedays/bootstrap/api/user/UserController.kt`)에서 상위 인터페이스에 선언된 헤더 변수(`xRegisterToken` 등)는 변수명이나 어노테이션을 변경하지 않고 그대로 사용합니다.
🔇 Additional comments (5)
bootstrap/api/src/main/kotlin/com/threedays/bootstrap/api/user/OASModelAdapter.kt (2)
46-52: 구현이 깔끔하고 명확합니다
도메인 모델에서 OAS 모델로의 변환이 잘 구현되어 있습니다.
18-22:
URL 변환 시 유효성 검증 추가 필요
url.toURI() 호출 시 잘못된 URL 형식에 대한 예외 처리가 없습니다. 유효하지 않은 URL이 입력될 경우 런타임 예외가 발생할 수 있습니다.
다음과 같이 안전한 URL 변환 로직을 추가하는 것을 제안합니다:
- url = this.url.toURI(),
+ url = runCatching { this.url.toURI() }
+ .getOrElse { throw IllegalArgumentException("Invalid URL format: ${this.url}", it) },bootstrap/api/src/main/kotlin/com/threedays/bootstrap/api/user/UserController.kt (3)
101-105: 도메인 모델 변환 로직이 잘 적용되었습니다
새로운 확장 함수들을 사용하여 코드가 더 깔끔하고 일관성 있게 정리되었습니다.
152-153: 응답 모델 변환이 일관성 있게 구현되었습니다
도메인 모델에서 응답 모델로의 변환이 새로운 확장 함수를 사용하여 일관성 있게 구현되었습니다.
176-176: 응답 변환이 적절하게 구현되었습니다
desiredPartner의 변환이 새로운 확장 함수를 사용하여 깔끔하게 처리되었습니다.
따로 선호하는 방식은 없고 object class로 해도 큰 상관 없을 것 같아요! 요렇게 수정해볼께요 |
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
bootstrap/api/src/main/kotlin/com/threedays/bootstrap/api/user/OASModelAdapter.kt (2)
47-53: 함수 반환 타입 명시로 가독성 향상
toOASModel함수의 반환 타입이 명시되어 있지 않습니다. 반환 타입을 명시하면 코드의 이해도를 높이고 컴파일러 에러를 사전에 방지할 수 있습니다.반환 타입을 명시적으로 지정해 주세요:
- fun toOASModel(domainModel: com.threedays.domain.user.entity.UserProfile) = UserProfile( + fun toOASModel(domainModel: com.threedays.domain.user.entity.UserProfile): UserProfile = UserProfile(
65-76:birthYearRange필드의 nullable 처리 주의
birthYearRange의start와end가 nullable로 처리되고 있습니다. 이를 사용하는 쪽에서 null 체크를 누락할 경우 문제가 발생할 수 있으므로 기본값을 설정하거나 nullable 임을 명확히 인지할 수 있도록 주석이나 문서화가 필요합니다.
start와end에 기본값을 지정하거나 null일 경우를 처리하는 로직을 추가하세요:birthYearRange = domainModel.birthYearRange?.let { BirthYearRange( start = it.start?.value ?: DEFAULT_START_YEAR, end = it.end?.value ?: DEFAULT_END_YEAR, ) }또는 nullable임을 명확히 하기 위해 코틀린의 nullable 타입을 적극 활용하세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
bootstrap/api/src/main/kotlin/com/threedays/bootstrap/api/user/OASModelAdapter.kt(1 hunks)bootstrap/api/src/main/kotlin/com/threedays/bootstrap/api/user/UserController.kt(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- bootstrap/api/src/main/kotlin/com/threedays/bootstrap/api/user/UserController.kt
bootstrap/api/src/main/kotlin/com/threedays/bootstrap/api/user/OASModelAdapter.kt
Show resolved
Hide resolved
bootstrap/api/src/main/kotlin/com/threedays/bootstrap/api/user/OASModelAdapter.kt
Show resolved
Hide resolved
Summary by CodeRabbit
New Features
Bug Fixes
Refactor