[threeten-dev] [threeten-develop] Review for minor update to ValueRange checkValidIntValue

[Stephen Colebourne]

I hate the inheritDoc change. Is it done this way anywhere else in the
JDK (I don't think so). While I accept it may be easier for
maintainers, it is far worse for most developers who are just reading
the Javadoc as source code (having hit F3 in Eclipse to jump to the
source code of the method they are working with). While the HTML tool
can hunt down the inheritDoc, regular users cannot.

The inlined code looks OK, however the Javadoc of the method is now
wrong as it specifies the implementation.

thanks
Stephen


On 5 March 2013 16:16, roger riggs <roger.riggs at oracle.com> wrote:
> Hi,
>
> Makes sense.
>
> I inlined the equivalent of checkValidIntValue in TemporalAccessor.get.
> The @throws clauses were slightly out of alignment with
> TemporalAccessor.get.
> To make it simplier to maintain the implementers now use {@inheritDoc}.
> The ValueRange.checkValidIntValue exception message is updated to more
> informative.
>
> Updated webrev:
>
> Webrev:  http://cr.openjdk.java.net/~rriggs/webrev-checkvalid-274/
> javadoc: http://cr.openjdk.java.net/~rriggs/javadoc-checkvalid-274/
>
> Thanks, Roger
>
> On 3/5/2013 8:49 AM, Stephen Colebourne wrote:
>> I don't think this change can go in.
>>
>> The checkValidValue methods are public, and so can be called by anyone
>> at any time in any way. The UTTE is intended for use when the Field or
>> Unit passed in is unsupported by the target object. In these methods,
>> the field is optional, and supported/not-supported is not a quality of
>> the ValueRange class. Thus UTTE is just plain wrong to be thrown here.
>>
>> A correct fix would be to inline the code change into TemporalAccesor
>> and anywhere else that has a similar use of the check method. If it is
>> widely used, a package scoped method (in an appropriate location) may
>> be a solution.
>>
>> thanks
>> Stephen
>>
>>
>>
>> On 4 March 2013 17:53, roger riggs <roger.riggs at oracle.com> wrote:
>>> Hi,
>>>
>>> A simple fix for the exception and message when TemporalAccessor.get()
>>> is used (incorrectly) to retrieve Fields with long values.
>>>
>>> Webrev:  http://cr.openjdk.java.net/~rriggs/webrev-checkvalid-274/
>>>
>>> Thanks, Roger
>>>
>>>
>
> ------------------------------------------------------------------------------
> Everyone hates slow websites. So do we.
> Make your web apps faster with AppDynamics
> Download AppDynamics Lite for free today:
> http://p.sf.net/sfu/appdyn_d2d_feb
> _______________________________________________
> threeten-develop mailing list
> threeten-develop at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/threeten-develop