[threeten-dev] [threeten-develop] Initial review of changes to support Hijrah variants

[roger riggs]

Hi,

On 2/12/2013 6:07 PM, Stephen Colebourne wrote:
> - The HijrahDate factory methods should specify which variant they create.
ok, as "Islamic Umm Al-Qura calendar"
>
> - HijrahChronology has Javadoc "CLDR Standard extension". It isn't
> obvious that is intended for use as a Locale extension.
Updated to refer to Locale extension as used by the Locale class.
>
> - HijrahChronology Javadoc refers to the Gregorian year/calendar,
> whereas everything else in 310 refers to the ISO year/calendar
changed to just epoch day/epoch month; some cleanup needed of description
of resource properties.
>
> - I think we may need to separate variants from chronologies in
> getAvailableChronologies(). The Julian-Gregorian cutover calendar
> (that we are not currently implementing) has a near-infinite number of
> variants. Registering all the variants isn't practical. While we
> aren't implementing this now, we might in the future, or we might
> implement some other calendar with lots of variants.
That's more like a parameter than a variant.
I would put the methods to modify parameters of a calendar on
the chronology itself where it is needed.

The LDML cv-xxx context does create a second level of lookup.
>
> I suggest:
> getAvailableChronologies() returns the default chronology
> a new instance (not static) method on Chronology
> getAvailableVariants() returns the variants of Hijrah
> Other calendar systems simply return a list of one (themselves) from
> getAvailableVariants().
> This would seem like a more scalable solution.
It would complicate the task of a UI that needed to assemble the list
of available calendars to be a two level iterative process.

BTW,  these calendar names need to be localized by the UI.
>
> - TestHijrahChronology suggests that the getId() method of
> HijrahChronology will return "Hijrah".
Its probably to be more explicit with Hijrah-umalqura; or something
similar and readable unless translation/locale lookup will occur before
presentation  formatting/parsing.
> - The comment of HijrahChronology.getId() says it returns "Hijrah"
> which isn't so.
fixed to be consistent
> I had assumed that HijrahChronology.INSTANCE would return getId() of
> "Hijrah-umalqura". However, it would be nicer from an ID perspective
> if it didn't, and only those returnes by getAvailableVariants() had
> the suffix.
That would mean different Chronology instances for Hijrah and 
Hijrah-umalqra;
but they are the same.   Can we live with the longer name (or make it 
easier to
read with mixed case, i.e.   Hijrah-Umm-Al-Qura
>
> - The range for YEAR vs YEAR_OF_ERA looks wrong. If there are two
> eras, then they cannot both have the same range. Have you actually
> removed era support? Or should HijrahDate.yearofEra field be renamed?
All of the expect variants only have data in the current ERA.

The Chronology.range method has no idea what Era is being queried
so it cannot be specific to the Era.  But since the variants only have
data for the AH era, that is used for the range for both YEAR and 
YEAR_OF_ERA.

So I think the BEFORE_AH era can be removed.
>
> - The checkValidDayOfYear() and similar methods can reuse code on
> ValueRange, although there may be a reason not to do this (eg
> performance)
Performance shouldn't be an issue; but is unmeasured.
>
> - HijrahEra can only refer to a hard-coded chronology without redesign AFAICT
Or the Era.date(), Era.DateYearDay(), Ea. are removed.
> - There is a broader problem of the lack of support for EPOCH_MONTH in non-ISO.
I don't see the value in it, I see only YearMonth as a consumer of that 
field.
Its not difficult to add but I don't see the benefit.

It is not needed for formatting;  DateTimeFormatting mentions it but
there is neither format letter nor a source of the field in parsing.
>
> - TestZoneTextPrinterParser has some commented out code. OK, but looks
> like some odd code layout formatting line 169.
Temporary removal;  It was not clear that test code should be producing
reams of output unrelated to test results.  I'll need to ask Sherman.


>
> I haven't reviewed the accuracy of the date calculations.
I imported a few matching dates from the original data sets on Umm al-Qura
but since it is table driven it needs a slightly different approach compared
to algorithms.

Thanks, Roger


>
> Stephen
>
>
> On 12 February 2013 18:38, roger riggs <roger.riggs at oracle.com> wrote:
>> Hi,
>>
>> The changes to support multiple Hijrah calendar variants resulting
>> in significant changes to the implementation of HijrahChronology,
>> HijrahDate,
>> and related classes.
>>
>> The variants are identified by new properties in lib/calendars.properties.
>> Each variant is defined by a properties file containing id, type, hijrah
>> start and
>> corresponding Gregorian start dates plus month lengths for every month.
>> The format and contents of the properties file is still being reviewed.
>>
>> Please review and comment:
>>     http://cr.openjdk.java.net/~rriggs/webrev-hijrah-variants/
>>
>> javadoc:  (only the Hijrah classes are updated)
>>     http://cr.openjdk.java.net/~rriggs/javadoc-hijrah-variants/
>>
>> The related issues are:
>>    # 95 Deviation Options for the Hijrah Calendar
>>    #118 Confirm calendar system type of Islamic
>>
>> Thanks, Roger
>>
>>
>> ------------------------------------------------------------------------------
>> Free Next-Gen Firewall Hardware Offer
>> Buy your Sophos next-gen firewall before the end March 2013
>> and get the hardware for free! Learn more.
>> http://p.sf.net/sfu/sophos-d2d-feb
>> _______________________________________________
>> threeten-develop mailing list
>> threeten-develop at lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/threeten-develop
>>
> ------------------------------------------------------------------------------
> Free Next-Gen Firewall Hardware Offer
> Buy your Sophos next-gen firewall before the end March 2013
> and get the hardware for free! Learn more.
> http://p.sf.net/sfu/sophos-d2d-feb
> _______________________________________________
> 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