Skip to content
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

[transform.math] Fix missing unit when result is 0 #507

Merged
merged 3 commits into from
Jul 25, 2023

Conversation

clinique
Copy link
Contributor

@clinique clinique commented Jul 24, 2023

Resolves #506

Signed-off-by: clinique <gael@lhopital.org>
@clinique clinique changed the title [Math transform] Code modification proposal following issue #506 [Math transform] Code modification proposal Jul 24, 2023
Comment on lines 44 to 51
try {
source = new QuantityType<>(sourceString);
} catch (IllegalArgumentException e) {
logger.warn("Input value '{}' could not be converted to a valid number", sourceString);
throw new TransformationException("Math Transformation can only be used with numeric inputs");
}
QuantityType<?> value;
String evaluated = sourceString;
try {
value = new QuantityType<>(valueString);
source = new QuantityType<>(evaluated);
evaluated = valueString;
value = new QuantityType<>(evaluated);
} catch (IllegalArgumentException e) {
logger.warn("Input value '{}' could not be converted to a valid number", valueString);
logger.warn("Input value '{}' could not be converted to a valid number", evaluated);
throw new TransformationException("Math Transformation can only be used with numeric inputs");
}
Copy link
Member

Choose a reason for hiding this comment

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

Why did you change that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To avoid having two catch catching the same thing....could be avoided.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reverted

Comment on lines 58 to 54
QuantityType<?> result = performCalculation(source, value);
return BigDecimal.ZERO.compareTo(result.toBigDecimal()) == 0 ? "0" : result.toString();
return performCalculation(source, value).toString();
Copy link
Member

Choose a reason for hiding this comment

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

I See why it works now, but I wonder why it was done like that before. Maybe to prevent a return value of 0.0000?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I was wondering the same, reason why I wanted your advice.

Signed-off-by: clinique <gael@lhopital.org>
J-N-K
J-N-K previously approved these changes Jul 25, 2023
@J-N-K J-N-K added the bug Something isn't working label Jul 25, 2023
@J-N-K J-N-K added this to the 3.2.18 milestone Jul 25, 2023
@J-N-K J-N-K merged commit 0af9236 into smarthomej:4.0.x Jul 25, 2023
2 checks passed
@J-N-K J-N-K changed the title [Math transform] Code modification proposal [transform.math] Fix missing unit when result is 0 Jul 25, 2023
J-N-K added a commit to J-N-K/addons that referenced this pull request Jul 25, 2023
* Code modification proposal following issue smarthomej#506

Signed-off-by: clinique <gael@lhopital.org>
Signed-off-by: Jan N. Klug <github@klug.nrw>
@clinique clinique deleted the math_transform branch August 20, 2023 00:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Math transform] Unit is ignored if result is 0
2 participants