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

Rename and pull-up unit isDateUnit/isTimeUnit #312

Closed jodastephen closed 11 years ago

jodastephen commented 11 years ago

We pulled up the date/time check methods from ChronoField to TemporalField, but didn't do the same for ChronoUnit to TemporalUnit.

Similarly, they should be renamed to isDateBased() and isTimeBased() for consistency.

This will allow some improvements in TemporalUnit.isSupportedBy implementations.

jodastephen commented 11 years ago 
# HG changeset patch
# User scolebourne
# Date 1369316240 -3600
# Node ID 2c5a20d02dcf713db57499eec476e0b3f115e315
# Parent  32edb634b3c33520cef0530d1e8255dce8b72f6e
Remove default methods from TemporalField

These cannot be inferred, so should not be default methods

diff --git a/src/share/classes/java/time/temporal/IsoFields.java b/src/share/classes/java/time/temporal/IsoFields.java
--- a/src/share/classes/java/time/temporal/IsoFields.java
+++ b/src/share/classes/java/time/temporal/IsoFields.java
@@ -545,6 +545,11 @@
         }

         @Override
+        public boolean isTimeBased() {
+            return false;
+        }
+
+        @Override
         public ValueRange rangeRefinedBy(TemporalAccessor temporal) {
             return range();
         }
diff --git a/src/share/classes/java/time/temporal/JulianFields.java b/src/share/classes/java/time/temporal/JulianFields.java
--- a/src/share/classes/java/time/temporal/JulianFields.java
+++ b/src/share/classes/java/time/temporal/JulianFields.java
@@ -253,6 +253,11 @@
         }

         @Override
+        public boolean isTimeBased() {
+            return false;
+        }
+
+        @Override
         public ValueRange range() {
             return range;
         }
diff --git a/src/share/classes/java/time/temporal/TemporalField.java b/src/share/classes/java/time/temporal/TemporalField.java
--- a/src/share/classes/java/time/temporal/TemporalField.java
+++ b/src/share/classes/java/time/temporal/TemporalField.java
@@ -106,15 +106,16 @@
     /**
      * Gets the display name for the field in the requested locale.
      * <p>
-     * If there is no display name for the locale the value of {@code getName}
-     * is returned.
+     * If there is no display name for the locale then a suitable default must be returned.
+     * <p>
+     * The default implementation must check the locale is not null
+     * and return {@code getName()}.
      *
      * @param locale  the locale to use, not null
-     * @return the display name for the locale or the value of {@code getName},
-     *     not null
+     * @return the display name for the locale or a suitable default, not null
      */
     default String getDisplayName(Locale locale) {
-        Objects.requireNonNull(locale, "local");
+        Objects.requireNonNull(locale, "locale");
         return getName();
     }

@@ -164,28 +165,24 @@
      * <p>
      * A field is date-based if it can be derived from
      * {@link ChronoField#EPOCH_DAY EPOCH_DAY}.
-     * <p>
-     * The default implementation must return false.
+     * Note that it is valid for both {@code isDateBased()} and {@code isTimeBased()}
+     * to return false, such as when representing a field like minute-of-week.
      *
      * @return true if this field is a component of a date
      */
-    default boolean isDateBased() {
-        return false;
-    }
+    boolean isDateBased();

     /**
      * Checks if this field represents a component of a time.
      * <p>
      * A field is time-based if it can be derived from
      * {@link ChronoField#NANO_OF_DAY NANO_OF_DAY}.
-     * <p>
-     * The default implementation must return false.
+     * Note that it is valid for both {@code isDateBased()} and {@code isTimeBased()}
+     * to return false, such as when representing a field like minute-of-week.
      *
      * @return true if this field is a component of a time
      */
-    default boolean isTimeBased() {
-        return false;
-    }
+    boolean isTimeBased();

     //-----------------------------------------------------------------------
     /**
diff --git a/src/share/classes/java/time/temporal/WeekFields.java b/src/share/classes/java/time/temporal/WeekFields.java
--- a/src/share/classes/java/time/temporal/WeekFields.java
+++ b/src/share/classes/java/time/temporal/WeekFields.java
@@ -897,6 +897,11 @@
         }

         @Override
+        public boolean isTimeBased() {
+            return false;
+        }
+
+        @Override
         public ValueRange range() {
             return range;
         }
diff --git a/test/java/time/tck/java/time/chrono/TCKChronoLocalDate.java b/test/java/time/tck/java/time/chrono/TCKChronoLocalDate.java
--- a/test/java/time/tck/java/time/chrono/TCKChronoLocalDate.java
+++ b/test/java/time/tck/java/time/chrono/TCKChronoLocalDate.java
@@ -459,6 +459,16 @@
         }

         @Override
+        public boolean isDateBased() {
+            return false;
+        }
+
+        @Override
+        public boolean isTimeBased() {
+            return false;
+        }
+
+        @Override
         public boolean isSupportedBy(TemporalAccessor temporal) {
             throw new UnsupportedOperationException("Not supported yet.");
         }
diff --git a/test/java/time/tck/java/time/chrono/TCKChronoLocalDateTime.java b/test/java/time/tck/java/time/chrono/TCKChronoLocalDateTime.java
--- a/test/java/time/tck/java/time/chrono/TCKChronoLocalDateTime.java
+++ b/test/java/time/tck/java/time/chrono/TCKChronoLocalDateTime.java
@@ -473,6 +473,16 @@
         }

         @Override
+        public boolean isDateBased() {
+            return false;
+        }
+
+        @Override
+        public boolean isTimeBased() {
+            return false;
+        }
+
+        @Override
         public boolean isSupportedBy(TemporalAccessor temporal) {
             throw new UnsupportedOperationException("Not supported yet.");
         }
diff --git a/test/java/time/tck/java/time/chrono/TCKChronoZonedDateTime.java b/test/java/time/tck/java/time/chrono/TCKChronoZonedDateTime.java
--- a/test/java/time/tck/java/time/chrono/TCKChronoZonedDateTime.java
+++ b/test/java/time/tck/java/time/chrono/TCKChronoZonedDateTime.java
@@ -475,6 +475,16 @@
         }

         @Override
+        public boolean isDateBased() {
+            return false;
+        }
+
+        @Override
+        public boolean isTimeBased() {
+            return false;
+        }
+
+        @Override
         public boolean isSupportedBy(TemporalAccessor temporal) {
             throw new UnsupportedOperationException("Not supported yet.");
         }
diff --git a/test/java/time/test/java/time/temporal/MockFieldNoValue.java b/test/java/time/test/java/time/temporal/MockFieldNoValue.java
--- a/test/java/time/test/java/time/temporal/MockFieldNoValue.java
+++ b/test/java/time/test/java/time/temporal/MockFieldNoValue.java
@@ -96,6 +96,16 @@
         return ValueRange.of(1, 20);
     }

+    @Override
+    public boolean isDateBased() {
+        return false;
+    }
+
+    @Override
+    public boolean isTimeBased() {
+        return false;
+    }
+
     //-----------------------------------------------------------------------
     @Override
     public boolean isSupportedBy(TemporalAccessor temporal) {
jodastephen commented 11 years ago 
# HG changeset patch
# User scolebourne
# Date 1369319816 -3600
# Node ID 625730ed90f5a82d3ff427b81b1f570c5992dbfb
# Parent  2c5a20d02dcf713db57499eec476e0b3f115e315
Add isDateBased/isTimeBased to TemporalUnit

Fixes #312

diff --git a/src/share/classes/java/time/LocalDateTime.java b/src/share/classes/java/time/LocalDateTime.java
--- a/src/share/classes/java/time/LocalDateTime.java
+++ b/src/share/classes/java/time/LocalDateTime.java
@@ -1616,8 +1616,7 @@
         }
         LocalDateTime end = (LocalDateTime) endDateTime;
         if (unit instanceof ChronoUnit) {
-            ChronoUnit f = (ChronoUnit) unit;
-            if (f.isTimeUnit()) {
+            if (unit.isTimeBased()) {
                 long amount = date.daysUntil(end.date);
                 if (amount == 0) {
                     return time.periodUntil(end.time, unit);
@@ -1630,7 +1629,7 @@
                     amount++;  // safe
                     timePart -= NANOS_PER_DAY;  // safe
                 }
-                switch (f) {
+                switch ((ChronoUnit) unit) {
                     case NANOS:
                         amount = Math.multiplyExact(amount, NANOS_PER_DAY);
                         break;
diff --git a/src/share/classes/java/time/LocalTime.java b/src/share/classes/java/time/LocalTime.java
--- a/src/share/classes/java/time/LocalTime.java
+++ b/src/share/classes/java/time/LocalTime.java
@@ -995,8 +995,7 @@
     @Override
     public LocalTime plus(long amountToAdd, TemporalUnit unit) {
         if (unit instanceof ChronoUnit) {
-            ChronoUnit f = (ChronoUnit) unit;
-            switch (f) {
+            switch ((ChronoUnit) unit) {
                 case NANOS: return plusNanos(amountToAdd);
                 case MICROS: return plusNanos((amountToAdd % MICROS_PER_DAY) * 1000);
                 case MILLIS: return plusNanos((amountToAdd % MILLIS_PER_DAY) * 1000_000);
diff --git a/src/share/classes/java/time/ZonedDateTime.java b/src/share/classes/java/time/ZonedDateTime.java
--- a/src/share/classes/java/time/ZonedDateTime.java
+++ b/src/share/classes/java/time/ZonedDateTime.java
@@ -1540,8 +1540,7 @@
     @Override
     public ZonedDateTime plus(long amountToAdd, TemporalUnit unit) {
         if (unit instanceof ChronoUnit) {
-            ChronoUnit u = (ChronoUnit) unit;
-            if (u.isDateUnit()) {
+            if (unit.isDateBased()) {
                 return resolveLocal(dateTime.plus(amountToAdd, unit));
             } else {
                 return resolveInstant(dateTime.plus(amountToAdd, unit));
@@ -2055,8 +2054,7 @@
         if (unit instanceof ChronoUnit) {
             ZonedDateTime end = (ZonedDateTime) endDateTime;
             end = end.withZoneSameInstant(zone);
-            ChronoUnit u = (ChronoUnit) unit;
-            if (u.isDateUnit()) {
+            if (unit.isDateBased()) {
                 return dateTime.periodUntil(end.dateTime, unit);
             } else {
                 return toOffsetDateTime().periodUntil(end.toOffsetDateTime(), unit);
diff --git a/src/share/classes/java/time/chrono/ChronoLocalDateTimeImpl.java b/src/share/classes/java/time/chrono/ChronoLocalDateTimeImpl.java
--- a/src/share/classes/java/time/chrono/ChronoLocalDateTimeImpl.java
+++ b/src/share/classes/java/time/chrono/ChronoLocalDateTimeImpl.java
@@ -361,10 +361,9 @@
             throw new DateTimeException("Unable to calculate amount as objects have different chronologies");
         }
         if (unit instanceof ChronoUnit) {
-            ChronoUnit f = (ChronoUnit) unit;
-            if (f.isTimeUnit()) {
+            if (unit.isTimeBased()) {
                 long amount = end.getLong(EPOCH_DAY) - date.getLong(EPOCH_DAY);
-                switch (f) {
+                switch ((ChronoUnit) unit) {
                     case NANOS: amount = Math.multiplyExact(amount, NANOS_PER_DAY); break;
                     case MICROS: amount = Math.multiplyExact(amount, MICROS_PER_DAY); break;
                     case MILLIS: amount = Math.multiplyExact(amount, MILLIS_PER_DAY); break;
diff --git a/src/share/classes/java/time/temporal/ChronoUnit.java b/src/share/classes/java/time/temporal/ChronoUnit.java
--- a/src/share/classes/java/time/temporal/ChronoUnit.java
+++ b/src/share/classes/java/time/temporal/ChronoUnit.java
@@ -233,7 +233,7 @@
      */
     @Override
     public boolean isDurationEstimated() {
-        return isDateUnit();
+        return isDateBased();
     }

     //-----------------------------------------------------------------------
@@ -242,7 +242,8 @@
      *
      * @return true if a date unit, false if a time unit
      */
-    public boolean isDateUnit() {
+    @Override
+    public boolean isDateBased() {
         return this.compareTo(DAYS) >= 0;
     }

@@ -251,7 +252,8 @@
      *
      * @return true if a time unit, false if a date unit
      */
-    public boolean isTimeUnit() {
+    @Override
+    public boolean isTimeBased() {
         return this.compareTo(DAYS) < 0;
     }

@@ -261,12 +263,6 @@
         if (this == FOREVER) {
             return false;
         }
-        if (temporal instanceof ChronoLocalDate) {
-            return isDateUnit();
-        }
-        if (temporal instanceof ChronoLocalDateTime || temporal instanceof ChronoZonedDateTime) {
-            return true;
-        }
         return TemporalUnit.super.isSupportedBy(temporal);
     }

diff --git a/src/share/classes/java/time/temporal/IsoFields.java b/src/share/classes/java/time/temporal/IsoFields.java
--- a/src/share/classes/java/time/temporal/IsoFields.java
+++ b/src/share/classes/java/time/temporal/IsoFields.java
@@ -656,6 +656,16 @@
         }

         @Override
+        public boolean isDateBased() {
+            return true;
+        }
+
+        @Override
+        public boolean isTimeBased() {
+            return false;
+        }
+
+        @Override
         public boolean isSupportedBy(Temporal temporal) {
             return temporal.isSupported(EPOCH_DAY);
         }
@@ -663,7 +673,7 @@
         @SuppressWarnings("unchecked")
         @Override
         public <R extends Temporal> R addTo(R temporal, long amount) {
-            switch(this) {
+            switch (this) {
                 case WEEK_BASED_YEARS:
                     return (R) temporal.with(WEEK_BASED_YEAR,
                             Math.addExact(temporal.get(WEEK_BASED_YEAR), amount));
@@ -692,7 +702,6 @@
         @Override
         public String toString() {
             return getName();
-
         }
     }
 }
diff --git a/src/share/classes/java/time/temporal/TemporalUnit.java b/src/share/classes/java/time/temporal/TemporalUnit.java
--- a/src/share/classes/java/time/temporal/TemporalUnit.java
+++ b/src/share/classes/java/time/temporal/TemporalUnit.java
@@ -63,7 +63,11 @@

 import java.time.DateTimeException;
 import java.time.Duration;
+import java.time.LocalTime;
 import java.time.Period;
+import java.time.chrono.ChronoLocalDate;
+import java.time.chrono.ChronoLocalDateTime;
+import java.time.chrono.ChronoZonedDateTime;

 /**
  * A unit of date-time, such as Days or Hours.
@@ -132,6 +136,33 @@

     //-----------------------------------------------------------------------
     /**
+     * Checks if this unit represents a component of a date.
+     * <p>
+     * A date is time-based if it can be used to imply meaning from a date.
+     * It must have a {@linkplain #getDuration() duration} that is an integral
+     * multiple of the length of a standard day.
+     * Note that it is valid for both {@code isDateBased()} and {@code isTimeBased()}
+     * to return false, such as when representing a unit like 36 hours.
+     *
+     * @return true if this unit is a component of a date
+     */
+    boolean isDateBased();
+
+    /**
+     * Checks if this unit represents a component of a time.
+     * <p>
+     * A unit is time-based if it can be used to imply meaning from a time.
+     * It must have a {@linkplain #getDuration() duration} that divides into
+     * the length of a standard day without remainder.
+     * Note that it is valid for both {@code isDateBased()} and {@code isTimeBased()}
+     * to return false, such as when representing a unit like 36 hours.
+     *
+     * @return true if this unit is a component of a time
+     */
+    boolean isTimeBased();
+
+    //-----------------------------------------------------------------------
+    /**
      * Checks if this unit is supported by the specified temporal object.
      * <p>
      * This checks that the implementing date-time can add/subtract this unit.
@@ -144,6 +175,15 @@
      * @return true if the unit is supported
      */
     default boolean isSupportedBy(Temporal temporal) {
+        if (temporal instanceof LocalTime) {
+            return isTimeBased();
+        }
+        if (temporal instanceof ChronoLocalDate) {
+            return isDateBased();
+        }
+        if (temporal instanceof ChronoLocalDateTime || temporal instanceof ChronoZonedDateTime) {
+            return true;
+        }
         try {
             temporal.plus(1, this);
             return true;
diff --git a/test/java/time/tck/java/time/TCKInstant.java b/test/java/time/tck/java/time/TCKInstant.java
--- a/test/java/time/tck/java/time/TCKInstant.java
+++ b/test/java/time/tck/java/time/TCKInstant.java
@@ -582,6 +582,14 @@
             return false;
         }
         @Override
+        public boolean isDateBased() {
+            return false;
+        }
+        @Override
+        public boolean isTimeBased() {
+            return true;
+        }
+        @Override
         public boolean isSupportedBy(Temporal temporal) {
             return false;
         }
@@ -609,6 +617,14 @@
             return false;
         }
         @Override
+        public boolean isDateBased() {
+            return false;
+        }
+        @Override
+        public boolean isTimeBased() {
+            return false;
+        }
+        @Override
         public boolean isSupportedBy(Temporal temporal) {
             return false;
         }
diff --git a/test/java/time/tck/java/time/TCKLocalTime.java b/test/java/time/tck/java/time/TCKLocalTime.java
--- a/test/java/time/tck/java/time/TCKLocalTime.java
+++ b/test/java/time/tck/java/time/TCKLocalTime.java
@@ -965,6 +965,14 @@
             return false;
         }
         @Override
+        public boolean isDateBased() {
+            return false;
+        }
+        @Override
+        public boolean isTimeBased() {
+            return true;
+        }
+        @Override
         public boolean isSupportedBy(Temporal temporal) {
             return false;
         }
@@ -992,6 +1000,14 @@
             return false;
         }
         @Override
+        public boolean isDateBased() {
+            return false;
+        }
+        @Override
+        public boolean isTimeBased() {
+            return false;
+        }
+        @Override
         public boolean isSupportedBy(Temporal temporal) {
             return false;
         }
diff --git a/test/java/time/tck/java/time/chrono/TCKChronoLocalDate.java b/test/java/time/tck/java/time/chrono/TCKChronoLocalDate.java
--- a/test/java/time/tck/java/time/chrono/TCKChronoLocalDate.java
+++ b/test/java/time/tck/java/time/chrono/TCKChronoLocalDate.java
@@ -412,6 +412,16 @@
         }

         @Override
+        public boolean isDateBased() {
+            return false;
+        }
+
+        @Override
+        public boolean isTimeBased() {
+            return false;
+        }
+
+        @Override
         public boolean isSupportedBy(Temporal temporal) {
             throw new UnsupportedOperationException("Not supported yet.");
         }
diff --git a/test/java/time/tck/java/time/chrono/TCKChronoLocalDateTime.java b/test/java/time/tck/java/time/chrono/TCKChronoLocalDateTime.java
--- a/test/java/time/tck/java/time/chrono/TCKChronoLocalDateTime.java
+++ b/test/java/time/tck/java/time/chrono/TCKChronoLocalDateTime.java
@@ -426,6 +426,16 @@
         }

         @Override
+        public boolean isDateBased() {
+            return false;
+        }
+
+        @Override
+        public boolean isTimeBased() {
+            return false;
+        }
+
+        @Override
         public boolean isSupportedBy(Temporal temporal) {
             throw new UnsupportedOperationException("Not supported yet.");
         }
diff --git a/test/java/time/tck/java/time/chrono/TCKChronoZonedDateTime.java b/test/java/time/tck/java/time/chrono/TCKChronoZonedDateTime.java
--- a/test/java/time/tck/java/time/chrono/TCKChronoZonedDateTime.java
+++ b/test/java/time/tck/java/time/chrono/TCKChronoZonedDateTime.java
@@ -428,6 +428,16 @@
         }

         @Override
+        public boolean isDateBased() {
+            return false;
+        }
+
+        @Override
+        public boolean isTimeBased() {
+            return false;
+        }
+
+        @Override
         public boolean isSupportedBy(Temporal temporal) {
             throw new UnsupportedOperationException("Not supported yet.");
         }
RogerRiggs commented 11 years ago

The changes look fine; please include an issue ID in every commit to help track changes.

jodastephen commented 11 years ago

Fiexd by http://hg.openjdk.java.net/threeten/threeten/jdk/rev/2c5a20d02dcf and http://hg.openjdk.java.net/threeten/threeten/jdk/rev/625730ed90f5