HijrahDate factories #259

Closed
RogerRiggs opened this Issue Feb 6, 2013 · 4 comments

Comments

Projects
None yet
2 participants
Contributor

RogerRiggs commented Feb 6, 2013

The HijrahDate class has static factories now(), date(y,m,d),... like the other date classes; but due to the support or deviations and multiple calendars they need to be reconsidered. Alternatives:

  1. Consider Umm al-Qura to be the default and specified to be used by the factory methods. The other variants would be accessible using Chronology.of(variant).
  2. Remove the factory methods from HijrahDate directing developers to use Chronology.of(variant).date(y,m,d)
  3. Add a parameter to the factory methods to identify the variant. For example, HijrahDate.now("islamic-sa0").
Owner

jodastephen commented Feb 6, 2013

I think I favour option 1. I mentioned before that Oracle will need to choose a default, and this was why. I think that the other thread has done this, as an input of "islamic" maps to a default in #118. In my view, once there is a default, the methods just document that they use it.

It may be worth adding a method withVariant(HijrahChronology) to HijrahDate to allow easy switching.

Option 2 would be unfriendly to casual users of HijrahDate, while option 3 feels overly heavy. There is an option 4 - a different date class for each variant, which is perhaps the "correct" way, but would no doubt break the generics.

Contributor

RogerRiggs commented Feb 12, 2013

ok, will add HijrahDate.withVariant(HijrahChronology); it will retain the year, month, and day unless in the new chronology the month is shorter in which case the day is set to the last day of the month.

Owner

jodastephen commented Feb 12, 2013

Approved

@ghost ghost assigned RogerRiggs Feb 19, 2013

@RogerRiggs RogerRiggs closed this Feb 21, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment