ThreeTen / threeten

This project was the home of code used to develop a modern date and time library for JDK8. Development has moved to OpenJDK and a separate backport project, threetenbp.
http://threeten.github.io/
191 stars 37 forks source link

ValueRange checkValidIntValue exceptions #274

Closed RogerRiggs closed 11 years ago

RogerRiggs commented 11 years ago

Related to issue #245... The ValueRange.checkValidIntValue method is used by the default TemporalAccessor.get method to validate the field and value but throws a different exception and message than the overridden implementations fixed in issue #245. The exceptions and message should be updated to be UnsupportedTemporalTypeException and DateTimeException.

jodastephen commented 11 years ago

Not sure that this is correct. The code says return range(field).checkValidIntValue(getLong(field), field);, and the range(field) part will throw UTTE if necessary, so the checkValidIntValue isn't significant.

RogerRiggs commented 11 years ago

If the field is supported then ChronoField.range() just returns the range object from the ChronoField.
If the field is not supported then UTTE is thrown that that's different than the check to see if the value of the field is valid according to checkInt. I only ran into it because I got the old inscrutable exception message when forgetting to use getLong.

jodastephen commented 11 years ago

This is done isn't it?

RogerRiggs commented 11 years ago

Almost, I've gotten interrupted before the final tweaks are in.

RogerRiggs commented 11 years ago

Resolved with http://hg.openjdk.java.net/threeten/threeten/jdk/rev/5a0759d1c8b9

jodastephen commented 11 years ago

Removing the "default implementation must" section means that the spec does not require what the default implementation does, thus subclasses cannot rely on it. The "default implementation must" does not have to include code, but it must be present.

RogerRiggs commented 11 years ago

It should be sufficient to mention the dependency on the range and getLong methods but the description now matches the style in the other methods. Resolve with http://hg.openjdk.java.net/threeten/threeten/jdk/rev/f1b828b14763