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

[roger riggs]

Hi,

Comments and updates...

javadoc: http://cr.openjdk.java.net/~rriggs/javadoc-temporaltypeexception-245/

webrev: http://cr.openjdk.java.net/~rriggs/webrev-temporaltypeexception-245/


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