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

Rename ParserException to MeasurementParseException #46

Closed
keilw opened this issue Jul 10, 2017 · 8 comments
Closed

Rename ParserException to MeasurementParseException #46

keilw opened this issue Jul 10, 2017 · 8 comments

Comments

@keilw
Copy link
Member

keilw commented Jul 10, 2017

A rather generic ParserException can be found at least in JDK Nashorn, but otherwise both the JDK and other JSRs either call it ParseException (like java.text.ParseException) or *ParseException (like MonetaryParseException, DateTimeParseException, etc.)

It is not the most urgent case, but with other proposals like #39 that also would require adding a new name and deprecate the old one, it's worth do mention this as well. A new exception could be called MeasurementParseException.

@keilw
Copy link
Member Author

keilw commented Jan 28, 2018

Removed usage of ParserException from a format() method, so I would leave it for now, and close this one. Leaving #58 open.

@keilw keilw closed this as completed Jan 28, 2018
@keilw keilw reopened this Feb 18, 2018
@keilw keilw added the vote label Feb 18, 2018
@keilw
Copy link
Member Author

keilw commented Feb 18, 2018

Reopened this as it would be cleaner and more consistent with JSR 310, 354 and others.

The idea would be to leave ParserException in the API 2.0 if we have to (for backward compatibility) but simply extend a new MeasurementParseException and mark ParserException deprecated.

Please vote on either:

@keilw keilw changed the title Rename ParserException to something like MeasurementParseException Rename ParserException to MeasurementParseException Feb 18, 2018
@keilw
Copy link
Member Author

keilw commented Mar 20, 2018

A) Rename ParserException to MeasurementParseException for consistency with a large number of APIs and frameworks (JSR 310, 354, Jackson, Gradle,...) marking the old one deprecated.

@keilw
Copy link
Member Author

keilw commented Mar 20, 2018

B) Keep ParserException as it is.

@dautelle
Copy link

Why not MeasureParseException (makes more sense and shorter) ?

@desruisseaux
Copy link
Contributor

But this exception can also occur while parsing Unit objects instead of Measure objects, isn't it?

@keilw
Copy link
Member Author

keilw commented Mar 26, 2018

Yes, but Measure is not an API element any more, Quantity is. Having the base exception start with Measurement, I thought MeasurementParseException (similar to DateTimeParseException in Date/Time) would be a good name. We have a QuantityFormat interface now, so both Unit and Quantity can be parsed, which is why UnitParseException won't be enough.

@keilw
Copy link
Member Author

keilw commented Mar 26, 2018

Although it is only an SPI element of Indriya, we have a Measurement interface ever since JSR 363, so calling it MeasurementParseException sounds more consequent since this exception will also extend MeasurementException. Compared to IncommensurableException it is just one more character ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants