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

Support parsing localized strings with DecimalType, PercentType and QuantityType #2365

Merged
merged 1 commit into from May 19, 2021

Conversation

wborn
Copy link
Member

@wborn wborn commented May 15, 2021

Adds constructors with a Locale parameter for parsing localized string to DecimalType, PercentType and QuantityType.
This allows for parsing locale specific group and decimal seperators in strings.

Also fixes:

  • IllegalArgumentException not being thrown if numbers in strings are only partially parsed as QuantityType
  • MeasurementParseException not caught and rethrown as IllegalArgumentException

Several new unit tests cover the new functionality and fixed regressions.


Further improves #2362

@wborn wborn added the enhancement An enhancement or new feature of the Core label May 15, 2021
@wborn wborn force-pushed the locale-number-parsing branch 4 times, most recently from 655c3b4 to 41a1bfa Compare May 18, 2021 22:24
…uantityType

Adds constructors with a Locale parameter for parsing localized string to DecimalType, PercentType and QuantityType.
This allows for parsing locale specific group and decimal seperators in strings.

Also fixes:
* IllegalArgumentException not being thrown if numbers in strings are only partially parsed as QuantityType
* MeasurementParseException not caught and rethrown as IllegalArgumentException

Several new unit tests cover the new functionality and fixed regressions.

Signed-off-by: Wouter Born <github@maindrain.net>
@wborn wborn marked this pull request as ready for review May 18, 2021 22:34
@wborn wborn requested a review from a team as a code owner May 18, 2021 22:34
@cweitkamp cweitkamp added the UoM Units of Measurement label May 19, 2021
@wborn wborn added the i18n/l10n Internationalization/Localization label May 19, 2021
Copy link
Member

@kaikreuzer kaikreuzer left a comment

Choose a reason for hiding this comment

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

lgtm, thanks!

@kaikreuzer kaikreuzer merged commit 3c30177 into openhab:main May 19, 2021
@kaikreuzer kaikreuzer added this to the 3.1 milestone May 19, 2021
@wborn wborn deleted the locale-number-parsing branch May 19, 2021 19:28
@wborn
Copy link
Member Author

wborn commented May 20, 2021

Seems like some add-ons expect different exceptions to be thrown for illegal arguments since some tests now fail:

https://ci.openhab.org/job/PR-openHAB-Addons/4221/#showFailuresLink

IMHO it is better to always throw the same exception (IllegalArgumentException, new behavior) instead just throwing whatever runtime exception is thrown by the underlying implementations (NumberFormatException, MeasurementParseException, etc.).
This behavior was never documented or covered with unit tests in the Core.

WDYT @openhab/core-maintainers?

@wborn
Copy link
Member Author

wborn commented May 20, 2021

Throwing a NumberFormatException could still make sense for the DecimalType and PercentType constructors. It extends the IllegalArgumentException and has as doc:

 * Thrown to indicate that the application has attempted to convert
 * a string to one of the numeric types, but that the string does not
 * have the appropriate format.

For the QuantityType it is a bit more difficult since it can have a unit, but the unit is optional.

@wborn
Copy link
Member Author

wborn commented May 20, 2021

Besides that, the PercentType also throws an IllegalArgumentException when the value is not in the [0, 100] range. So to be on the safe side, when using these constructors, you probably want to catch any IllegalArgumentException.

fwolter pushed a commit to fwolter/openhab-core that referenced this pull request May 24, 2021
…uantityType (openhab#2365)

Adds constructors with a Locale parameter for parsing localized string to DecimalType, PercentType and QuantityType.
This allows for parsing locale specific group and decimal seperators in strings.

Also fixes:
* IllegalArgumentException not being thrown if numbers in strings are only partially parsed as QuantityType
* MeasurementParseException not caught and rethrown as IllegalArgumentException

Several new unit tests cover the new functionality and fixed regressions.

Signed-off-by: Wouter Born <github@maindrain.net>
splatch pushed a commit to ConnectorIO/copybara-hab-core that referenced this pull request Jul 11, 2023
…uantityType (openhab#2365)

Adds constructors with a Locale parameter for parsing localized string to DecimalType, PercentType and QuantityType.
This allows for parsing locale specific group and decimal seperators in strings.

Also fixes:
* IllegalArgumentException not being thrown if numbers in strings are only partially parsed as QuantityType
* MeasurementParseException not caught and rethrown as IllegalArgumentException

Several new unit tests cover the new functionality and fixed regressions.

Signed-off-by: Wouter Born <github@maindrain.net>
GitOrigin-RevId: 3c30177
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement An enhancement or new feature of the Core i18n/l10n Internationalization/Localization UoM Units of Measurement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants