-
Notifications
You must be signed in to change notification settings - Fork 0
Feat/#12 WispRegistry 코드 생성 기능 구현 #13
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
…rresponding route factory class names
angryPodo
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.
정말 고생많으셨습니다🙇🏻♂️ 코드가 점점 읽기 편해지는게 보여서 저도 많이 배우고 가요!
사소한 제안들 남겼습니다, 한번 확인해주세요💪🏻
| val factoriesProperty = PropertySpec.builder("factories", mapType) | ||
| .addModifiers(KModifier.INTERNAL) | ||
| .initializer(initializerBlock.build()) | ||
| .build() |
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.
생성 로직 잘 구현해주셨네요! 고생하셨습니다🥲
여기서 한가지 제안은 WispRegistry 내부에 registry라는 이름의 private 맵을 만들고 이 맵을 사용하는 createRoute같은 public 함수를 만드는건 어떨까요?
위의 제안은 향후에 외부에서 registry맵에 직접 접근하는 것을 막아서 더 안정적인 구조를 만들 수 있을 것 같습니다👍🏻
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.
좋은 아이디어 같습니다👍 레지스트리 객체를 생성하는 것에만 집중하다 보니 캡슐화와 유지보수 측면을 놓쳤던 것 같습니다. 말씀하신 대로 registry를 private으로 숨기고 접근 메서드를 제공하는 구조가 훨씬 안정적이겠네요. 바로 반영하겠습니다!
| private fun validateDuplicatePaths( | ||
| routesWithSymbols: List<Pair<RouteInfo, KSClassDeclaration>> | ||
| ): Boolean { |
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.
이 부분도 구조적인 측면에서 제안이 있어요😊
이 검증 함수를 WispValidator클래스로 옮기는 건 어떻게 생각하시나요?
저는 WispProcessor는 심볼을 찾고 생성기를 호출하는 흐름을 담당하고 Wispvalidator는 모든 종류의 유효성 검사를 책임지는 구조가 될것 같아요🤔
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.
좋은 제안입니다! 😊 작성할 때 기존에 있던 validateSerializable 때문에 검증 로직을 어디에 두는 게 맞을지 저도 살짝 고민했었거든요. ㅎㅎ 제안해주신 대로 WispValidator 객체에서 유효성 검사를 할 수 있도록 리팩토링 진행하겠습니다!
| val factoryObject = TypeSpec.objectBuilder(routeInfo.factoryClassName) | ||
| .addModifiers(KModifier.INTERNAL) | ||
| .addSuperinterface(routeFactoryInterface) | ||
| .addSuperinterface(ROUTE_FACTORY) |
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.
이거 아주 좋네요👍🏻
WispError관련 ClassName도 옮겨도 좋다고 생각해요!
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.
바로 수정하겠습니다!
Summary
수집된 모든
RouteFactory정보를 관리하는WispRegistry싱글톤 객체 생성 로직을 구현하고, 빌드 타임에 중복된 경로를 감지하는 검증 로직을 추가했습니다.Related Issue
Closes #12
ScreenShot(Option)
Details
이번 PR은 개별
RouteFactory들을 하나로 묶어 런타임에 제공하는 Registry 시스템을 구축합니다.WispRegistryGenerator구현kotlinpoet을 사용하여WispRegistry코드를 생성합니다.path와RouteFactory를 매핑하는Map을 생성하여 런타임에서O(1)조회가 가능하도록 했습니다.WispProcessor로직 통합process단계에서 수집된 모든RouteInfo를 모아 마지막에 레지스트리를 생성하도록 흐름을 개선했습니다.Dependencies(aggregating = true)를 설정하여 증분 빌드 시 올바르게 갱신되도록 했습니다.중복 검사 구현
@Wisp("path")를 가질 경우,logger를 통해 해당 심볼 위치에 에러를 표시하도록 추가했습니다.테스트 코드 추가
WispRegistryGeneratorTest: 생성된 코드가 예상된Map구조를 갖는지 문자열 검증WispClassName 추가
WispRegistryGenerator와RouteFactoryGenerator에서 공통적으로 사용되는ClassName을 추출하여 관리하도록 수정했습니다.