Extend ScaledBigDecimal class with Number - #64
Conversation
Piiit
left a comment
Piiit
left a comment
There was a problem hiding this comment.
LGTM, but I would create a ticket to clean stuff up later on
| Class<?> type | ||
| ) { | ||
| if (!recordClass.isRecord()) { | ||
| if (!recordClass.isRecord() && !ScaledBigDecimal.class.isAssignableFrom(recordClass)) { |
There was a problem hiding this comment.
OK for now, but I think we should create a ticket to clean this up. It seems strange to have a specific implementation inside this generic utility.
There was a problem hiding this comment.
I agree. We should have a ticket to clean up the ultility. And another ticket to look for other tools that use this to see if they can be improved. I am sure there are other places what will be happy about ScaledBigDecimal finally implementing Number.
There was a problem hiding this comment.
Done 💪🏼
AB-490 Spring Boot Toolbox: Clean up RecordReflectionUtil
AB-491 Spring Boot Toolbox: Check for ScaledBigDecimal extends Number refactoring possibilities in projects
3 participants
Note
I will backport this to v1 once this is approved.
The abstract class
java.lang.Numberrequires the following methods to be implemented, which we already implemented inScaledBigDecimal, so the contract is already in place.I hope that strictly using records for custom types, which cannot extends other classes, was not the reason that we did not extend
NumberforScaledBigDecimal.