-
Notifications
You must be signed in to change notification settings - Fork 0
[Refactor/#169] 도메인 모듈 내 클래스 정리 #170
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
base: develop
Are you sure you want to change the base?
Conversation
Walkthrough도메인 내 Time/Date 클래스를 Java 8의 LocalTime/LocalDate로 통일하고, BitnagilError의 패키지를 개요날짜/시간 도메인 클래스(Time, Date)를 Java 8의 LocalTime, LocalDate로 통일하고, BitnagilError를 domain.common 패키지로 이동하는 대규모 리팩토링. 저장소 계층, 도메인 계층, 프레젠테이션 계층 전반에 걸쳐 타입 및 import 경로 업데이트. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@presentation/src/main/java/com/threegap/bitnagil/presentation/writeroutine/model/Date.kt`:
- Around line 53-63: fromString() can produce invalid Date instances that cause
LocalDate.of(...) in toLocalDate() / checkInRange() to throw DateTimeException;
fix by validating input and handling construction failures: in fromString() (the
factory/parser) verify the split has exactly 3 parts, parse year/month/day with
safe integer parsing, then either (a) validate month in 1..12 and day in 1..31
and attempt to build a LocalDate inside a try/catch to catch DateTimeException,
or (b) return a nullable/Result/throw a clear IllegalArgumentException on
invalid parse so callers know creation failed. Also consider making
toLocalDate() either handle DateTimeException (wrap and rethrow with context) or
return LocalDate? / optional to avoid unexpected runtime crashes; ensure
checkInRange() uses the validated/constructed LocalDate only after successful
creation.
- Around line 14-18: In the Date.now() factory function, avoid calling
LocalDate.now() three times; instead call LocalDate.now() once, store the result
in a local val (e.g., now) and extract year/month/day from that single instance
so year/month/day can't come from different dates; update the companion/object
function Date.now to use the single captured LocalDate.
presentation/src/main/java/com/threegap/bitnagil/presentation/writeroutine/model/Date.kt
Outdated
Show resolved
Hide resolved
presentation/src/main/java/com/threegap/bitnagil/presentation/writeroutine/model/Date.kt
Show resolved
Hide resolved
wjdrjs00
left a comment
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.
LGTM~👍🏻
[ PR Content ]
도메인 모듈 내 날자/시간 관련된 클래스를 LocalDate, LocalTime 기반으로 통일합니다.
도메인 모듈 내 공통적으로 사용되는 BitnagilError 클래스의 경로를 common/model으로 이동합니다.
Related issue
Screenshot 📸
x
Work Description
To Reviewers 📢
domain/common/error로 하고자 하였으나, 도메인 모듈 내 타 패키지의 model 폴더에서 별도의 하위 폴더 없이 바로 클래스를 배치하고 있어domain/common에 배치했습니다Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.