-
Notifications
You must be signed in to change notification settings - Fork 0
Feat#10 RouteFactory 코드 생성 기능 구현 #11
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
Conversation
…erate route factories
…sor with codeGenerator and logger
…and maintainability
… route info types
… classes for object and class route types
…outes based on class kind
…ameter handling and validation
…ameter handling and validation
jm991014
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.
@Wisp 어노테이션이 붙은 심볼을 분석하여 RouteFactory를 생성하는 핵심 로직이 아주 잘 구현된 것 같네요ㅎㅎ 특히 Mapper와 Generator의 책임을 명확히 분리한 설계 덕분에 코드를 이해하기 쉽고, 꼼꼼한 테스트 코드를 통해 생성 로직을 단위 테스트로 검증한 점이 인상적입니다. 👍
고생하셨습니다!!
|
|
||
| private fun generateRouteFactory( | ||
| routeClass: KSClassDeclaration, | ||
| routeInfo: com.angrypodo.wisp.model.RouteInfo |
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.
여기는 별도로 import하지 않은 이유가 있나요?
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.
아 아니요 잘못들어갔네요🥲
요즘 일각고래 버전으로 올린 다음부터 이런 현상이 꽤 있네요. 수정하겠습니다ㅎㅎ
| routeInfo: com.angrypodo.wisp.model.RouteInfo | ||
| ) { | ||
| val fileSpec = factoryGenerator.generate(routeInfo) | ||
| val dependencies = Dependencies(true, routeClass.containingFile!!) |
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.
현재 process 메서드 내부에서 각각의 심볼에 대해 해당 함수를 호출하는 것으로 보입니다.
하나의 심볼에 대한 RouteFactory를 생성하는 경우기 때문에 Dependencies의 aggregating 인자를 false로 설정하여 증분 빌드 시 불필요한 재처리를 방지하고 빌드 성능을 최적화하는 것은 어떨까요?🙋♂️
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.
아 증분빌드는 생각하지 못했네요. 어차피 심볼이 독립적으로 처리되니 훨씬 최적화에 유리할 것 같습니다ㅎㅎ 반영하겠습니다👍🏻
| nonNullableType == INT -> CodeBlock.of("%L?.toIntOrNull()", rawAccess) | ||
| nonNullableType == LONG -> CodeBlock.of("%L?.toLongOrNull()", rawAccess) | ||
| nonNullableType == BOOLEAN -> CodeBlock.of("%L?.toBooleanStrictOrNull()", rawAccess) | ||
| nonNullableType == FLOAT -> CodeBlock.of("%L?.toFloatOrNull()", rawAccess) |
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.
요기 Double은 따로 지원하지 않는 이유가 있나요?
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.
추가적으로 지원하지 않는 타입에 대해서 예외를 던지기보다는 KSPLogger로 처리하는건 어떻게 생각하시나요?
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.
요기 Double은 따로 지원하지 않는 이유가 있나요?
Long만 고려하고 Double은 누락했네요😅 바로 추가하도록 하겠습니다!
추가적으로 지원하지 않는 타입에 대해서 예외를 던지기보다는 KSPLogger로 처리하는건 어떻게 생각하시나요?
좋은 생각이네요!! 오류가 발생한 위치를 알려주는게 사용자 경험 측면에서 훨씬 좋을 것 같습니다ㅎㅎ 이것도 리팩토링 하겠습니다~!
Summary
@Wisp어노테이션이 붙은@Serializable라우트 클래스를 분석하여, 딥링크 파라미터를 변환하고 라우트 객체를 생성하는RouteFactory코드를 자동으로 생성하는 기능을 구현했습니다.Related Issue
Closes #10
ScreenShot(Option)
Details
RouteFactory코드 생성 기능 구현RouteFactoryGenerator를 구현하여, 각 라우트 클래스에 대한RouteFactory오브젝트를 생성합니다.String,Int,Long,Boolean,Float,Enum및 각 타입의Nullable버전을 포함한 모든 필수 파라미터 타입을 지원합니다.data object라우트 지원 및 리팩토링RouteInfo모델을sealed interface로 리팩토링하여,data class와data object타입을 명확하게 구분하도록 개선했습니다.data object에 대해서는 생성자 호출 없이 객체 자체를 반환(= Home)하도록 수정했습니다.유닛 테스트 코드 수정 및 정상화
KotlinPoet의 코드 생성 특성(Expression body, Simple Name 사용)을 반영하여, 깨지기 쉬웠던 문자열 비교 방식의 테스트 검증 로직을 모두 수정했습니다.코드 품질 개선
WispProcessor의 매핑 로직을RouteInfoMapper로 분리하여 단일 책임 원칙을 준수하도록 구조를 개선했습니다.