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

[roger riggs]

Hi Stephen,

The description of the default implementation is redundant with
the requirements described in the exceptions and is unnecessary.

Roger

On 3/7/2013 5:11 AM, Stephen Colebourne wrote:
> The Javadoc in TemporalAccessor still needs fixing:
>
> * Implementations must not alter either this object.
>   * <p>
>   * The default implementation must behave equivalent to this code:
>   * <pre>
>   *  return range(field).checkValidIntValue(getLong(field), field);
>
> The first line is a typo. The second part describes the default
> implementation, and thus needs adjusting to match the new
> implementation.
>
> Stephen
>
>
>
> On 6 March 2013 21:27, roger riggs <roger.riggs at oracle.com> wrote:
>> Reverted the javadoc for @inheritDoc; the overridden methods all have
>> the same @throws clauses.
>>
>> Updated webrev:
>>    http://cr.openjdk.java.net/~rriggs/webrev-checkvalid-274/
>>
>> Roger
>>
>>
>>
>> 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.
>>
>> 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
>>
>>
>> ------------------------------------------------------------------------------
>> Symantec Endpoint Protection 12 positioned as A LEADER in The Forrester
>> Wave(TM): Endpoint Security, Q1 2013 and "remains a good choice" in the
>> endpoint security space. For insight on selecting the right partner to
>> tackle endpoint security challenges, access the full report.
>> http://p.sf.net/sfu/symantec-dev2dev
>> _______________________________________________
>> threeten-develop mailing list
>> threeten-develop at lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/threeten-develop
>>
>>
>> --
>> Thanks, Roger
>>
>> Oracle Java Platform Group
>>
>> Oracle is committed to developing practices and products that help protect
>> the environment
>>
>>
>>
>> ------------------------------------------------------------------------------
>> Symantec Endpoint Protection 12 positioned as A LEADER in The Forrester
>> Wave(TM): Endpoint Security, Q1 2013 and "remains a good choice" in the
>> endpoint security space. For insight on selecting the right partner to
>> tackle endpoint security challenges, access the full report.
>> http://p.sf.net/sfu/symantec-dev2dev
>> _______________________________________________
>> threeten-develop mailing list
>> threeten-develop at lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/threeten-develop
>>
> ------------------------------------------------------------------------------
> Symantec Endpoint Protection 12 positioned as A LEADER in The Forrester
> Wave(TM): Endpoint Security, Q1 2013 and "remains a good choice" in the
> endpoint security space. For insight on selecting the right partner to
> tackle endpoint security challenges, access the full report.
> http://p.sf.net/sfu/symantec-dev2dev
> _______________________________________________
> threeten-develop mailing list
> threeten-develop at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/threeten-develop

-- 
Thanks, Roger

Oracle Java Platform Group

Green Oracle <http://www.oracle.com/commitment> Oracle is committed to 
developing practices and products that help protect the environment