search

ThreeTen / threeten

This project was the home of code used to develop a modern date and time library for JDK8. Development has moved to OpenJDK and a separate backport project, threetenbp.
http://threeten.github.io/
191 stars 37 forks source link

Aligned week field implementations in JapaneseChronology #319

Closed jodastephen closed 11 years ago

jodastephen commented 11 years ago

The JapaneseChronology does not really support the aligned week fields at present, however the "not supported" status isn't well documented or implemented. We should either implement the fields properly, or properly disable them. Personally, I'm tempted to disable them, given the complications around day-of-year in that calendar.

RogerRiggs commented 11 years ago

Documenting and disbling them seems reasonable. Generally, we found little or no support for week-of-year in Japan.

jodastephen commented 11 years ago

Patch to fix this, also change some bugs to be TODOs and enable some more tests

# HG changeset patch
# User scolebourne
# Date 1372440541 -3600
# Node ID 9855c77c0e55a21cf9a649731042e1432718f33f
# Parent  cfba4fe9fdc0fad53be5f40fa1c43ef9c8a3156c
Remove all support for week-based fields
See #319

diff --git a/src/share/classes/java/time/chrono/JapaneseChronology.java b/src/share/classes/java/time/chrono/JapaneseChronology.java
--- a/src/share/classes/java/time/chrono/JapaneseChronology.java
+++ b/src/share/classes/java/time/chrono/JapaneseChronology.java
@@ -77,6 +77,7 @@
 import java.time.temporal.TemporalAccessor;
 import java.time.temporal.TemporalAdjuster;
 import java.time.temporal.TemporalField;
+import java.time.temporal.UnsupportedTemporalTypeException;
 import java.time.temporal.ValueRange;
 import java.util.Arrays;
 import java.util.Calendar;
@@ -96,7 +97,7 @@
  * apart from the era-based year numbering.
  * <p>
  * Only Meiji (1865-04-07 - 1868-09-07) and later eras are supported.
- * Older eras are handled as an unknown era where the year-of-era is the ISO year.
+ * Older eras are handled using the era 'Seireki' where the year-of-era is the ISO year.
  *
  * @implSpec
  * This class is immutable and thread-safe.
@@ -104,7 +105,6 @@
  * @since 1.8
  */
 public final class JapaneseChronology extends Chronology implements Serializable {
-    // TODO: definition for unknown era may break requirement that year-of-era >= 1

     static final LocalGregorianCalendar JCAL =
         (LocalGregorianCalendar) CalendarSystem.forName("japanese");
@@ -386,54 +386,34 @@
     @Override
     public ValueRange range(ChronoField field) {
         switch (field) {
-            case YEAR:
-            case DAY_OF_MONTH:
-            case DAY_OF_WEEK:
-            case MICRO_OF_DAY:
-            case MICRO_OF_SECOND:
-            case HOUR_OF_DAY:
-            case HOUR_OF_AMPM:
-            case MINUTE_OF_DAY:
-            case MINUTE_OF_HOUR:
-            case SECOND_OF_DAY:
-            case SECOND_OF_MINUTE:
-            case MILLI_OF_DAY:
-            case MILLI_OF_SECOND:
-            case NANO_OF_DAY:
-            case NANO_OF_SECOND:
-            case CLOCK_HOUR_OF_DAY:
-            case CLOCK_HOUR_OF_AMPM:
-            case EPOCH_DAY:
-            case PROLEPTIC_MONTH:
-            case MONTH_OF_YEAR:
-                return field.range();
-            case ERA:
-                return ValueRange.of(JapaneseEra.SEIREKI.getValue(),
-                                     getCurrentEra().getValue());
-        }
-        Calendar jcal = Calendar.getInstance(LOCALE);
-        int fieldIndex;
-        switch (field) {
+            case ALIGNED_DAY_OF_WEEK_IN_MONTH:
+            case ALIGNED_DAY_OF_WEEK_IN_YEAR:
+            case ALIGNED_WEEK_OF_MONTH:
+            case ALIGNED_WEEK_OF_YEAR:
+                throw new UnsupportedTemporalTypeException("Unsupported field: " + field);
             case YEAR_OF_ERA: {
+                Calendar jcal = Calendar.getInstance(LOCALE);
                 int startYear = getCurrentEra().getPrivateEra().getSinceDate().getYear();
                 return ValueRange.of(Year.MIN_VALUE, jcal.getGreatestMinimum(Calendar.YEAR),
                         jcal.getLeastMaximum(Calendar.YEAR) + 1, // +1 due to the different definitions
-                        Year.MAX_VALUE - startYear);
+                        Year.MAX_VALUE - startYear);  // TODO: Maximum is end of Seireki
             }
-            case DAY_OF_YEAR:
-                fieldIndex = Calendar.DAY_OF_YEAR;
-                break;
+            case DAY_OF_YEAR: {
+                Calendar jcal = Calendar.getInstance(LOCALE);
+                int fieldIndex = Calendar.DAY_OF_YEAR;
+                return ValueRange.of(jcal.getMinimum(fieldIndex), jcal.getGreatestMinimum(fieldIndex),
+                        jcal.getLeastMaximum(fieldIndex), jcal.getMaximum(fieldIndex));
+            }
+            case ERA:
+                return ValueRange.of(JapaneseEra.SEIREKI.getValue(), getCurrentEra().getValue());
             default:
-                 // TODO: review the remaining fields
-                throw new UnsupportedOperationException("Unimplementable field: " + field);
+                return field.range();
         }
-        return ValueRange.of(jcal.getMinimum(fieldIndex), jcal.getGreatestMinimum(fieldIndex),
-                jcal.getLeastMaximum(fieldIndex), jcal.getMaximum(fieldIndex));
     }

     //-----------------------------------------------------------------------
     @Override  // override for return type
-    public JapaneseDate resolveDate(Map<TemporalField, Long> fieldValues, ResolverStyle resolverStyle) {
+    public JapaneseDate resolveDate(Map <TemporalField, Long> fieldValues, ResolverStyle resolverStyle) {
         return (JapaneseDate) super.resolveDate(fieldValues, resolverStyle);
     }

diff --git a/src/share/classes/java/time/chrono/JapaneseDate.java b/src/share/classes/java/time/chrono/JapaneseDate.java
--- a/src/share/classes/java/time/chrono/JapaneseDate.java
+++ b/src/share/classes/java/time/chrono/JapaneseDate.java
@@ -56,6 +56,10 @@
  */
 package java.time.chrono;

+import static java.time.temporal.ChronoField.ALIGNED_DAY_OF_WEEK_IN_MONTH;
+import static java.time.temporal.ChronoField.ALIGNED_DAY_OF_WEEK_IN_YEAR;
+import static java.time.temporal.ChronoField.ALIGNED_WEEK_OF_MONTH;
+import static java.time.temporal.ChronoField.ALIGNED_WEEK_OF_YEAR;
 import static java.time.temporal.ChronoField.DAY_OF_MONTH;
 import static java.time.temporal.ChronoField.DAY_OF_YEAR;
 import static java.time.temporal.ChronoField.MONTH_OF_YEAR;
@@ -71,6 +75,7 @@
 import java.time.LocalDate;
 import java.time.LocalTime;
 import java.time.Period;
+import java.time.Year;
 import java.time.ZoneId;
 import java.time.temporal.ChronoField;
 import java.time.temporal.TemporalAccessor;
@@ -204,8 +209,11 @@
      *  or if the date is not a Japanese era
      */
     public static JapaneseDate of(JapaneseEra era, int yearOfEra, int month, int dayOfMonth) {
-        // TODO: handle SEIREKI
         Objects.requireNonNull(era, "era");
+        if (era == JapaneseEra.SEIREKI) {
+            // TODO: handle SEIREKI year validation
+            return new JapaneseDate(LocalDate.of(yearOfEra, month, dayOfMonth));
+        }
         LocalGregorianCalendar.Date jdate = JapaneseChronology.JCAL.newCalendarDate(null);
         jdate.setEra(era.getPrivateEra()).setDate(yearOfEra, month, dayOfMonth);
         if (!JapaneseChronology.JCAL.validate(jdate)) {
@@ -262,7 +270,11 @@
      *  or if the day-of-year is invalid for the year
      */
     static JapaneseDate ofYearDay(JapaneseEra era, int yearOfEra, int dayOfYear) {
-        // TODO: handle SEIREKI
+        Objects.requireNonNull(era, "era");
+        if (era == JapaneseEra.SEIREKI) {
+            // TODO: handle SEIREKI year validation
+            return new JapaneseDate(LocalDate.ofYearDay(yearOfEra, dayOfYear));
+        }
         CalendarDate firstDay = era.getPrivateEra().getSinceDate();
         LocalGregorianCalendar.Date jdate = JapaneseChronology.JCAL.newCalendarDate(null);
         jdate.setEra(era.getPrivateEra());
@@ -369,26 +381,44 @@

     @Override
     public int lengthOfYear() {
-        Calendar jcal = Calendar.getInstance(JapaneseChronology.LOCALE);
-        jcal.set(Calendar.ERA, era.getValue() + JapaneseEra.ERA_OFFSET);
-        jcal.set(yearOfEra, isoDate.getMonthValue() - 1, isoDate.getDayOfMonth());
-        return  jcal.getActualMaximum(Calendar.DAY_OF_YEAR);
+        if (getEra() == JapaneseEra.SEIREKI) {
+            return isoDate.lengthOfYear();
+        } else {
+            Calendar jcal = Calendar.getInstance(JapaneseChronology.LOCALE);
+            jcal.set(Calendar.ERA, era.getValue() + JapaneseEra.ERA_OFFSET);
+            jcal.set(yearOfEra, isoDate.getMonthValue() - 1, isoDate.getDayOfMonth());
+            return  jcal.getActualMaximum(Calendar.DAY_OF_YEAR);
+        }
     }

     //-----------------------------------------------------------------------
     @Override
+    public boolean isSupported(TemporalField field) {
+        if (field == ALIGNED_DAY_OF_WEEK_IN_MONTH || field == ALIGNED_DAY_OF_WEEK_IN_YEAR ||
+                field == ALIGNED_WEEK_OF_MONTH || field == ALIGNED_WEEK_OF_YEAR) {
+            return false;
+        }
+        return ChronoLocalDate.super.isSupported(field);
+    }
+
+    @Override
     public ValueRange range(TemporalField field) {
         if (field instanceof ChronoField) {
             if (isSupported(field)) {
                 ChronoField f = (ChronoField) field;
                 switch (f) {
-                    case DAY_OF_MONTH:
-                    case ALIGNED_WEEK_OF_MONTH:
-                        return isoDate.range(field);
-                    case DAY_OF_YEAR:
-                        return actualRange(field);
-                    case YEAR_OF_ERA:
-                        return actualRange(field);
+                    case DAY_OF_MONTH: return ValueRange.of(1, lengthOfMonth());
+                    case DAY_OF_YEAR: return ValueRange.of(1, lengthOfYear());
+                    case YEAR_OF_ERA: {
+                        if (getEra() == JapaneseEra.SEIREKI) {
+                            return ValueRange.of(Year.MIN_VALUE, JapaneseEra.MEIJI.getPrivateEra().getSinceDate().getYear());
+                        } else {
+                            Calendar jcal = Calendar.getInstance(JapaneseChronology.LOCALE);
+                            jcal.set(Calendar.ERA, era.getValue() + JapaneseEra.ERA_OFFSET);
+                            jcal.set(yearOfEra, isoDate.getMonthValue() - 1, isoDate.getDayOfMonth());
+                            return ValueRange.of(1, jcal.getActualMaximum(Calendar.YEAR));
+                        }
+                    }
                 }
                 return getChronology().range(f);
             }
@@ -397,43 +427,20 @@
         return field.rangeRefinedBy(this);
     }

-    private ValueRange actualRange(TemporalField field) {
-        if (getEra() == JapaneseEra.SEIREKI) {
-            return isoDate.range(field);
-        } else {
-            Calendar jcal = Calendar.getInstance(JapaneseChronology.LOCALE);
-            jcal.set(Calendar.ERA, era.getValue() + JapaneseEra.ERA_OFFSET);
-            jcal.set(yearOfEra, isoDate.getMonthValue() - 1, isoDate.getDayOfMonth());
-            return ValueRange.of(1, jcal.getActualMaximum(asCalendarField(field)));
-        }
-    }
-
-    /**
-     * Convert a ChronoField to a java.util.Calendar field value.
-     * @param field a ChronoField instance
-     * @return the corresponding java.util.Calendar field
-     * @throws IllegalStateException if an unused value is passed.
-     */
-    private int asCalendarField(TemporalField field) {
-        if (field == DAY_OF_YEAR) {
-            return Calendar.DAY_OF_YEAR;
-        } else if (field == YEAR_OF_ERA) {
-            return Calendar.YEAR;
-        } else {
-            throw new IllegalStateException("No conversion provided");
-        }
-    }
-
     @Override
     public long getLong(TemporalField field) {
         if (field instanceof ChronoField) {
             // same as ISO:
-            // DAY_OF_WEEK, ALIGNED_DAY_OF_WEEK_IN_MONTH, DAY_OF_MONTH, EPOCH_DAY,
-            // ALIGNED_WEEK_OF_MONTH, MONTH_OF_YEAR, PROLEPTIC_MONTH, YEAR
+            // DAY_OF_WEEK, DAY_OF_MONTH, EPOCH_DAY, MONTH_OF_YEAR, PROLEPTIC_MONTH, YEAR
             //
             // calendar specific fields
-            // ALIGNED_DAY_OF_WEEK_IN_YEAR, DAY_OF_YEAR, ALIGNED_WEEK_OF_YEAR, YEAR_OF_ERA, ERA
+            // DAY_OF_YEAR, YEAR_OF_ERA, ERA
             switch ((ChronoField) field) {
+                case ALIGNED_DAY_OF_WEEK_IN_MONTH:
+                case ALIGNED_DAY_OF_WEEK_IN_YEAR:
+                case ALIGNED_WEEK_OF_MONTH:
+                case ALIGNED_WEEK_OF_YEAR:
+                    throw new UnsupportedTemporalTypeException("Unsupported field: " + field);
                 case YEAR_OF_ERA:
                     return yearOfEra;
                 case ERA:
@@ -447,7 +454,6 @@
                         jcal.set(yearOfEra, isoDate.getMonthValue() - 1, isoDate.getDayOfMonth());
                         return jcal.get(Calendar.DAY_OF_YEAR);
                     }
-                // TODO: ALIGNED_DAY_OF_WEEK_IN_YEAR and ALIGNED_WEEK_OF_YEAR ???
             }
             return isoDate.getLong(field);
         }
@@ -477,7 +483,7 @@
     public JapaneseDate with(TemporalField field, long newValue) {
         if (field instanceof ChronoField) {
             ChronoField f = (ChronoField) field;
-            if (getLong(f) == newValue) {
+            if (getLong(f) == newValue) {  // getLong() validates for supported fields
                 return this;
             }
             switch (f) {
@@ -497,7 +503,6 @@
                 }
             }
             // YEAR, PROLEPTIC_MONTH and others are same as ISO
-            // TODO: review other fields, such as WEEK_OF_YEAR
             return with(isoDate.with(field, newValue));
         }
         return ChronoLocalDate.super.with(field, newValue);
diff --git a/test/java/time/tck/java/time/chrono/TCKJapaneseChronology.java b/test/java/time/tck/java/time/chrono/TCKJapaneseChronology.java
--- a/test/java/time/tck/java/time/chrono/TCKJapaneseChronology.java
+++ b/test/java/time/tck/java/time/chrono/TCKJapaneseChronology.java
@@ -178,12 +178,12 @@
                 {JapaneseEra.SHOWA, 1928 - YDIFF_SHOWA, 12, 25, 360, LocalDate.of(1928, 12, 25)},
                 {JapaneseEra.TAISHO, 1916 - YDIFF_TAISHO, 7, 30, 212, LocalDate.of(1916, 7, 30)},

-//                {JapaneseEra.SEIREKI, -100, 1, 1, 1, LocalDate.of(-100, 1, 1)},
-//                {JapaneseEra.SEIREKI, -1, 1, 1, 1, LocalDate.of(-1, 1, 1)},
-//                {JapaneseEra.SEIREKI, 0, 1, 1, 1, LocalDate.of(0, 1, 1)},
-//                {JapaneseEra.SEIREKI, 1, 1, 1, 1, LocalDate.of(1, 1, 1)},
-//                {JapaneseEra.SEIREKI, 1868, 9, 7, 251, LocalDate.of(1868, 9, 7)},
-//                {JapaneseEra.MEIJI, 1, 9, 8, 1, LocalDate.of(1868, 9, 8)},
+                {JapaneseEra.SEIREKI, -100, 1, 1, 1, LocalDate.of(-100, 1, 1)},
+                {JapaneseEra.SEIREKI, -1, 1, 1, 1, LocalDate.of(-1, 1, 1)},
+                {JapaneseEra.SEIREKI, 0, 1, 1, 1, LocalDate.of(0, 1, 1)},
+                {JapaneseEra.SEIREKI, 1, 1, 1, 1, LocalDate.of(1, 1, 1)},
+                {JapaneseEra.SEIREKI, 1868, 9, 7, 251, LocalDate.of(1868, 9, 7)},
+//                {JapaneseEra.MEIJI, 1, 9, 8, 1, LocalDate.of(1868, 9, 8)},  // TODO era definition differs
                 {JapaneseEra.MEIJI, 45, 7, 29, 211, LocalDate.of(1912, 7, 29)},
                 {JapaneseEra.TAISHO, 1, 7, 30, 1, LocalDate.of(1912, 7, 30)},
                 {JapaneseEra.TAISHO, 15, 12, 24, 358, LocalDate.of(1926, 12, 24)},
@@ -656,9 +656,6 @@
     public void test_getDayOfYear() {
         // Test all the Eras
         for (JapaneseEra era : JapaneseEra.values()) {
-            if (era == JapaneseEra.SEIREKI) {
-                continue;   // skip (until SEIREKI is well defined)
-            }
             JapaneseDate hd1 = JapaneseChronology.INSTANCE.dateYearDay(era, 1, 1);
             ValueRange range = hd1.range(DAY_OF_YEAR);
             assertEquals(range.getMaximum(), hd1.lengthOfYear(), "lengthOfYear should match range.getMaximum()");
RogerRiggs commented 11 years ago

looks ok

jodastephen commented 11 years ago

http://hg.openjdk.java.net/threeten/threeten/jdk/rev/4467c647ade9