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

LocalTime plus/minus DAYS #310

Closed jodastephen closed 11 years ago

jodastephen commented 11 years ago

Issue #70 added the ability for LocalTime to plus//minus days. This made sense on the basis that otherwise plus(24, HOURS) is allowed, but not plus(1, DAYS).

However, this is now the only case of this type remaining (previously there were more, like DayOfWeek and MonthOfYear). The special case isn't really justied any more particularly now Period and Duration are very separate.

jodastephen commented 11 years ago
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
@@ -974,9 +974,6 @@
      *  Returns a {@code LocalTime} with the specified number of half-days added.
      *  This is equivalent to {@link #plusHours(long)} with the amount
      *  multiplied by 12.
-     * <li>{@code DAYS} -
-     *  Returns a {@code LocalTime} with the specified number of days added.
-     *  This returns {@code this} time.
      * </ul>
      * <p>
      * All other {@code ChronoUnit} instances will throw an {@code UnsupportedTemporalTypeException}.
@@ -1007,7 +1004,6 @@
                 case MINUTES: return plusMinutes(amountToAdd);
                 case HOURS: return plusHours(amountToAdd);
                 case HALF_DAYS: return plusHours((amountToAdd % 2) * 12);
-                case DAYS: return this;
             }
             throw new UnsupportedTemporalTypeException("Unsupported unit: " + unit.getName());
         }
diff --git a/src/share/classes/java/time/temporal/Temporal.java b/src/share/classes/java/time/temporal/Temporal.java
--- a/src/share/classes/java/time/temporal/Temporal.java
+++ b/src/share/classes/java/time/temporal/Temporal.java
@@ -247,10 +247,6 @@
      * a date representing the 31st January, then adding one month would be unclear.
      * In cases like this, the field is responsible for resolving the result. Typically it will choose
      * the previous valid date, which would be the last valid day of February in this example.
-     * <p>
-     * If the implementation represents a date-time that has boundaries, such as {@code LocalTime},
-     * then the permitted units must include the boundary unit, but no multiples of the boundary unit.
-     * For example, {@code LocalTime} must accept {@code DAYS} but not {@code WEEKS} or {@code MONTHS}.
      *
      * @implSpec
      * Implementations must check and handle all units defined in {@link ChronoUnit}.
@@ -322,10 +318,6 @@
      * a date representing the 31st March, then subtracting one month would be unclear.
      * In cases like this, the field is responsible for resolving the result. Typically it will choose
      * the previous valid date, which would be the last valid day of February in this example.
-     * <p>
-     * If the implementation represents a date-time that has boundaries, such as {@code LocalTime},
-     * then the permitted units must include the boundary unit, but no multiples of the boundary unit.
-     * For example, {@code LocalTime} must accept {@code DAYS} but not {@code WEEKS} or {@code MONTHS}.
      *
      * @implSpec
      * Implementations must behave in a manor equivalent to the default method behavior.
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
@@ -142,7 +142,7 @@

     private static final TemporalUnit[] INVALID_UNITS;
     static {
-        EnumSet<ChronoUnit> set = EnumSet.range(WEEKS, FOREVER);
+        EnumSet<ChronoUnit> set = EnumSet.range(DAYS, FOREVER);
         INVALID_UNITS = (TemporalUnit[]) set.toArray(new TemporalUnit[set.size()]);
     }

@@ -1122,14 +1122,6 @@
         }
     }

-    @Test
-    public void test_plus_longTemporalUnit_multiples() {
-        assertEquals(TEST_12_30_40_987654321.plus(0, DAYS), TEST_12_30_40_987654321);
-        assertEquals(TEST_12_30_40_987654321.plus(1, DAYS), TEST_12_30_40_987654321);
-        assertEquals(TEST_12_30_40_987654321.plus(2, DAYS), TEST_12_30_40_987654321);
-        assertEquals(TEST_12_30_40_987654321.plus(-3, DAYS), TEST_12_30_40_987654321);
-    }
-
     @Test(expectedExceptions=NullPointerException.class)
     public void test_plus_longTemporalUnit_null() {
         TEST_12_30_40_987654321.plus(1, (TemporalUnit) null);
@@ -1556,14 +1548,6 @@
         }
     }

-    @Test
-    public void test_minus_longTemporalUnit_long_multiples() {
-        assertEquals(TEST_12_30_40_987654321.minus(0, DAYS), TEST_12_30_40_987654321);
-        assertEquals(TEST_12_30_40_987654321.minus(1, DAYS), TEST_12_30_40_987654321);
-        assertEquals(TEST_12_30_40_987654321.minus(2, DAYS), TEST_12_30_40_987654321);
-        assertEquals(TEST_12_30_40_987654321.minus(-3, DAYS), TEST_12_30_40_987654321);
-    }
-
     @Test(expectedExceptions=NullPointerException.class)
     public void test_minus_longTemporalUnit_null() {
         TEST_12_30_40_987654321.minus(1, (TemporalUnit) null);
RogerRiggs commented 11 years ago

Looks fine

jodastephen commented 11 years ago

Fixed by http://hg.openjdk.java.net/threeten/threeten/jdk/rev/dc15b61252bf