Skip to content
This repository has been archived by the owner on May 7, 2020. It is now read-only.

fix state conversion between HSBType and DecimalType #2504

Merged
merged 2 commits into from
Nov 22, 2016

Conversation

sjsf
Copy link
Contributor

@sjsf sjsf commented Nov 21, 2016

It looks like #2471 was caused by the missing conversion from DecimalType to HSBType. It got lost in #2334 because it was very well hidden in the super classes (i.e. PercentType/DimmerItem).

fixes #2471
Signed-off-by: Simon Kaufmann simon.kfm@googlemail.com

fixes eclipse-archived#2471
Signed-off-by: Simon Kaufmann <simon.kfm@googlemail.com>
@@ -165,6 +165,9 @@ public State as(Class<? extends State> target) {
} else {
return UnDefType.UNDEF;
}
} else if (target == HSBType.class) {
return new HSBType(DecimalType.ZERO, PercentType.ZERO,
new PercentType(this.toBigDecimal().multiply(new BigDecimal(100))));
Copy link
Contributor

@watou watou Nov 21, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BigDecimal.valueOf(100) is preferred to new BigDecimal(100): "This 'static factory method' is provided in preference to a (long) constructor because it allows for reuse of frequently used BigDecimal values." (link) (another instance above also)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@watou Thanks for that hint, didn't know that. But this valueOf talks about the constructor using a long. Does this argument also fit to ints (I need to look at the code why the static method is better then the constructor.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code shows that any ints between 0 and 10, inclusive, are better to use the valueOf(long) static factory method to avoid unnecessary construction of new BigDecimals. But remaining ignorant of the current implementation of valueOf and just going by the JavaDoc's advice seems sensible, should later/other implementations ever cache other values outside that range based on frequent usage (however unlikely we think that might be). I think there is no valueOf(int) static factory method because the int is just widened to a long without side effects.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An alternative approach to minimizing the number of duplicate immutable objects in the heap, but with more certainty of outcome, would be to define and use a static member HUNDRED like

private static final BigDecimal HUNDRED = BigDecimal.valueOf(100);

Signed-off-by: Simon Kaufmann <simon.kfm@googlemail.com>
@kaikreuzer kaikreuzer merged commit f7aa4bc into eclipse-archived:master Nov 22, 2016
@kaikreuzer
Copy link
Contributor

Many thanks @SJKA!

@rene54321
Copy link

Thanks a lot for fixing the failure. If I update now to the latest snaphot is it than implemented allready?

@kaikreuzer
Copy link
Contributor

No, you have to wait for this fix to be part of the next ESH stable build, see https://hudson.eclipse.org/smarthome/job/SmartHomeDistribution-Stable/changes.
All OH distros that are built AFTER that will contain the fix.

@sjsf sjsf deleted the fixDecimalHSBTypeConversion branch November 22, 2016 09:50
@kaikreuzer kaikreuzer added this to the 0.9.0 milestone Nov 30, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hue group issue since latest snapshot
5 participants