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

[roger riggs]

Hi Stephen,

On 3/6/2013 3:58 AM, Stephen Colebourne wrote:
> 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.
I count 94 classes in src/share/classes/java that use {@inheridDoc}, 
mostly for
exception text on @throws clauses.  (mostly in AWT and in collections.)

I don't usually develop with the JDK source (as an application developer),
there is too much a chance of depending on an implementation,
not the spec but I can see the point.

We also have quite a number of subclass methods that just say @Override
and have no source for the javadoc at all.

I can put them back but to me it is more useful to know that the semantics
of the exceptions are the unchanged (subclasses rarely change the 
exception semantics).

>
> The inlined code looks OK, however the Javadoc of the method is now
> wrong as it specifies the implementation.
I don't see the difference from the previous text.
The conditions from separate @throws have been combined.
Both new and old text define the conditions under which the exception
is thrown and which exception.

Roger

>
> 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
>>>>
>>>>