[threeten-dev] [threeten-develop] Draft of new UnsupportedTemporalTypeException update

[Stephen Colebourne]

On 25 February 2013 19:51, roger riggs <roger.riggs at oracle.com> wrote:
> javadoc:
> http://cr.openjdk.java.net/~rriggs/javadoc-temporaltypeexception-245/
> webrev: http://cr.openjdk.java.net/~rriggs/webrev-temporaltypeexception-245/

Much closer to what I was expecting ;-)

LocalDateTime almost certainly wants some Javadoc additions of UTTE.

None of the changes in HijrahChronology look right to me.

The check for era is FooEra should not be UTTE as UTTE needs to be
strictly scoped. (comment applies to all chronos) If you are desparate
to use a JDK exception, then ClassCastException would be OK there.

HijrahEra/IsoEra/MinguoEra has an import but no references to it.

In general, such as on TemporalAmount, DateTimeException should be
phrased in the general "throws DTE if unable to obtain the foo",
whereas UTTE is more specific "throws UTTE if the field/unit is
unsupported"

CopticDate/CopticEra still have too many UTTE.

Hope that helps
thanks
Stephen



> On 2/23/2013 6:15 PM, Stephen Colebourne wrote:
>>
>> This exception should definitely be in the temporal package.
>
> Moved to java.time.temporal
>
>>
>> It was intended that it should only be for those exceptions which
>> currently have the text "Unsuported field" or unit. (where the field
>> or unit must be an argument to the method)
>> So a number of examples don't make sense, DayOfWeek.of(),
>> Instant.create(), Instant.periodUntil() (first exception), and so on
>> (sadly, the change includes way too many exceptions)
>>
>> Basically, it should be:
>> - range(Field)
>> - get(Field)
>> - getLong(Field)
>> - with(Field, long)
>> - plus(long, Unit)
>> - minus(long, Unit)
>> - periodUntil(Temporal, Unit) - errors to do with the units validity
>
> Yes, if the field or unit is a parameter, explicitly or as 'this' and will
> be passed to
> a temporal which may reject it;  as in ChronoUnit.between().  These methods
> should be documented as being able to throw
> UnsupportedTemporalTypeException.
>
>
>>
>> The error relates specifically to the isSupported(Field) method and
>> the Unit.isSupportedBy() method.
>>
>> Other errors, to do with validating dates, checking temporal
>> classes/instanceof, are DateTimeException.
>> For example
>> - creating a date/time
>> - adjustInto
>> - periodUntil instanceof same type checks
>> - ZoneRegion/ZoneOffset
>> - ValueRange/checkValidValue
>
> Any method that accepts an unqualified Temporal will also throw
> this exception if the field is not supported and it should be documented.
> This includes adjustInto, rangeRefinedBy, getFrom, and compare.
>
> (btw, checkValidValue parameter field says it can be null but will NPE if it
> is).
>
> In the methods where an Era is passed but a specific Era subtype is
> required, the definition of UTTE could include this mismatch of the expected
> type.
> The alternative  would be ClassCastException.  DTE is too generic
>
>>
>> If the exceptions are going in signatures, then it has to be
>> consistent. Every method that is altered in Temporal/TemporalAccessor
>> must have the exception copied to everywhere that the same Javadoc has
>> been applied on all subclasses.
>
> yes
>
>>
>> In addition, the Javadoc words themselves specify DateTimeException,
>> which may also require changes.
>
> Fixed
>
>>
>> Any reference to "a DateTimeException" that changes to "a
>> UnsupportedTe..." also needs to change "a" to "an".
>>
>> It is important to ensure that methods can still throw
>> DateTimeException. For example, TemporalAmount.get is now only
>> permitted to throw UTTE, which prevents more unusual implementatons
>> that calculate their values on the fly.
>
> How should the description for DTE differ than that of UTTE?
> DTEs would not also be UTTE?
>
>>
>> To be honest, because there are way too many changes in this patch, it
>> may be easier to start again with more minimal changes hitting the
>> rules I've outlined above.
>> thanks
>
> The diff in Netbeans makes it easy to review the differences and revert as
> needed.
> (Sometimes I've found it easier to review the jdk.patch file than the
> individual diffs.)
>
> Thanks, Roger
>
>
>> Stephen
>>
>>
>>
>> On 23 February 2013 20:18, roger riggs <roger.riggs at oracle.com> wrote:
>>>
>>> Hi,
>>>
>>> Issue #245: https://github.com/ThreeTen/threeten/issues/245
>>> suggests that a more specific exception be used for cases
>>> where the field or units or values are unsupported or invalid.
>>>
>>> I expect that with some review, the criteria for using the new exception
>>> vs DateTimeException and the addition of @throws
>>> UnsupportedTemporalTypeException
>>> will need to be refined.
>>>
>>> It may be preferable to put the detailed exception into the
>>> java.time.temporal
>>> package since developers would normally only need to be using
>>> DateTimeException
>>> if they were catching exceptions.
>>>
>>> javadoc:
>>> http://cr.openjdk.java.net/~rriggs/javadoc-temporaltypeexception-245/
>>>
>>> webrev:
>>> http://cr.openjdk.java.net/~rriggs/webrev-temporaltypeexception-245/
>>>
>>> Please review and comment.
>>>
>>> --
>>> Thanks, Roger
>>>
>