Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
docs: add guide to using value classes #707
docs: add guide to using value classes #707
Changes from 1 commit
49aaaae
512a600
0eb4a45
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
value class가 java로 컴파일될 때 boxing과 unboxing 메소드가 생성되어서 이걸 호출해야 한다고 생각했는데, property를 통해서 접근하면 되는 거군요.
그런데 혹시 조회의 경우는 Spring에서 boxing 해서 반환해주나요?
위 Projection의 결과로 UseId를 얻을 수 있나 궁금해서요.
만약 된다면 Spring에서 구현한 코드를 통해서 가능한 것 같은데, 그렇다면 value class 지원은 Spring 기반에서만 동작하기 때문에 문서 최상단에
Spring을 사용하실 경우
와 같이 한정자가 들어가야 할 것 같아서요.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.
조회 시에 boxing 해서 반환하네요. 다만 boxing 하는 주체가 Spring인지 Hibernate인지는 추가적으로 확인해보겠습니다!
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.
java로 트랜스파일 한 결과에서는 long으로 변환되기에 jdsl의 findAll 호출 결과에는 long으로 담기는걸 확인했습니다.
boxing이 되는 위치는 코틀린 코드에서 해당 필드를 참조하는 곳이고, Java로 트랜스파일 시 boxing하는 코드를 코틀린이 넣어주는거 같아요.
만약 value class를 nullable로 선언하면 long 타입 대신 RecipeId가 그대로 유지되고,
이 경우 Hibernate에서 에러가 발생합니다.
다만 value class가 nullable인 경우는 value class 를 사용하는 의미가 없어지는 것 같아 해당 케이스는 고려 대상이 아닌 것 같습니다.
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.
더불어 답변 주신 내용에서 spring 이라고 말씀해주신 이유가 어떤 내용인지도 조금 더 설명해주실 수 있을까요?
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.
@erie0210 안녕하세요! 상세하게 확인해주셔서 감사합니다!
먼저 Spring을 언급한 이유는 Spring JPA에서 value class를 지원하게 되면서 Kotlin JDSL도 Spring과 같이 지원이 가능할지 고민하게 되었기 때문입니다.
다음으로 nullable한 value class도 충분히 사용하는 의미가 있다고 생각합니다. value class는 기본 자료형에 추가적인 validation이나 메소드들을 추가하기 위한 것으로 사용할 수 있기 때문입니다. BookPrice라는 value class가 있고 Book에 supplyPrice라는 필드가 있는데, 이 supplyPrice 필드가 nullable일 수 있다고 생각해요.
개인적으로는 nullable한 value class 또한 지원되어야 완벽한 지원이라 생각이 들기 떄문에, DTO Projection의 경우는 value class를 완벽하게 지원하지 않는 것으로 확인된 것 같습니다.
가이드 문서에 DTO Projection의 경우는 완벽한 지원이 안 되기 때문에 직접 value class를 사용하는 것보다, DTO Projection에서는 기본 자료형을 사용하고 조회 후에 map 등으로 변환을 추천한다는 내용을 추가해주실 수 있으신가요?
마지막으로 가이드 문서를 너무 잘 작성해주셔서 여러가지 부탁드리게 되네요. 도움을 주셔서 너무 감사합니다.
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.
말씀해주신 내용 반영했습니다.
map으로 변환하는 경우 class를 두 개 선언하는 불편함이 있을 것 같아
DTO class에서 value class로 변환하도록 예시 코드를 작성해봤는데 어떨까요? @shouwn
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.
@erie0210 오 좋다고 생각합니다!