Convert group function results to PercentType if DecimalType is not s… #3758
Conversation
…upported Fixes eclipse-archived#3734 And converts the test cases from Groovy to Java Signed-off-by: Stefan Triller <stefan.triller@telekom.de>
| State calculatedState = function.calculate(members); | ||
|
|
||
| // functions return DecimalTypes so we have to convert them if the baseItem type only supports PercentType | ||
| if (baseItem.getAcceptedDataTypes().contains(PercentType.class) |
There was a problem hiding this comment.
Wouldn't it make sense to check the type of calculatedState first?
if (calculatedState instanceof DecimalType && !baseItem.getAcceptedDataTypes().contains(DecimalType.class) ...)
| if (function != null && baseItem != null) { | ||
| State calculatedState = function.calculate(members); | ||
|
|
||
| // functions return DecimalTypes so we have to convert them if the baseItem type only supports PercentType |
There was a problem hiding this comment.
This seems to be a pretty specific solution...
Is there anything speaking against using ItemUtil.convertToAcceptedState(State, Item) here?
There was a problem hiding this comment.
Good idea, this method is exactly what i was looking for, but I was not aware of that it exists ;) I have changed it now.
|
I have included the converted test in this PR because it adds a new test for the implemented fix, see this test method |
Signed-off-by: Stefan Triller <stefan.triller@telekom.de>
There was a problem hiding this comment.
Although I have to second @maggu2810 that it would have been better to create a separate PR (or at least a separate commit) for the test migration, the change itself looks good to me now.
@maggu2810 okay for you too?
Okay for me. Personally I would use a one-liner and drop the variable declarations, but all fine... |
…upported
Fixes #3734
And converts the test cases from Groovy to Java
Signed-off-by: Stefan Triller stefan.triller@telekom.de