You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by ba...@apache.org on 2006/12/20 07:10:26 UTC

svn commit: r488926 - in /jakarta/commons/proper/lang/trunk/src: java/org/apache/commons/lang/time/DurationFormatUtils.java test/org/apache/commons/lang/time/DurationFormatUtilsTest.java

Author: bayard
Date: Tue Dec 19 22:10:26 2006
New Revision: 488926

URL: http://svn.apache.org/viewvc?view=rev&rev=488926
Log:
More tests, more bugfixes (aka rewrite of the guts). 

It's looking much better, the only edge case that throws it for a loop is if things start on the 29th of February in a year. I've hacked it in the day mode, but I'm not sure why I had to do that - however I trust the brute force test to be right in day mode. 
In month mode, it's even trickier as to what the correct answer is. How many months between 29th Feb and 28th of Feb the next year? The answer is 11, or with days included it's 11 months and 28 days. I can't see any reason to define that better, so I'm declaring that law. 

Things are weird if you start on Feb 29 :)

Modified:
    jakarta/commons/proper/lang/trunk/src/java/org/apache/commons/lang/time/DurationFormatUtils.java
    jakarta/commons/proper/lang/trunk/src/test/org/apache/commons/lang/time/DurationFormatUtilsTest.java

Modified: jakarta/commons/proper/lang/trunk/src/java/org/apache/commons/lang/time/DurationFormatUtils.java
URL: http://svn.apache.org/viewvc/jakarta/commons/proper/lang/trunk/src/java/org/apache/commons/lang/time/DurationFormatUtils.java?view=diff&rev=488926&r1=488925&r2=488926
==============================================================================
--- jakarta/commons/proper/lang/trunk/src/java/org/apache/commons/lang/time/DurationFormatUtils.java (original)
+++ jakarta/commons/proper/lang/trunk/src/java/org/apache/commons/lang/time/DurationFormatUtils.java Tue Dec 19 22:10:26 2006
@@ -273,11 +273,12 @@
     public static String formatPeriod(long startMillis, long endMillis, String format, boolean padWithZeros, 
             TimeZone timezone) {
 
-        long millis = endMillis - startMillis;
-        if (millis < 28 * DateUtils.MILLIS_PER_DAY) {
-            return formatDuration(millis, format, padWithZeros);
-        }
-
+        // Used to optimise for differences under 28 days and 
+        // called formatDuration(millis, format); however this did not work 
+        // over leap years. 
+        // TODO: Compare performance to see if anything was lost by 
+        // losing this optimisation. 
+        
         Token[] tokens = lexx(format);
 
         // timezones get funky around 0, so normalizing everything to GMT 
@@ -313,66 +314,73 @@
             hours += 24;
             days -= 1;
         }
-        // TODO: Create a test to see if this should be while. ie) one that makes hours above 
-        //       overflow and pushes this above the maximum # of days in a month?
-        int leapDays = 0;
-        if (days < 0) {
-            days += start.getActualMaximum(Calendar.DAY_OF_MONTH);
-            // Multiple answers possible. 
-            // For example, for Jan 15th to March 10th. If I count days-first it is 
-            // 1 month and 26 days, but if I count month-first then it is 1 month and 23 days.
-            // Here we choose the former. 
-            months -= 1;
-            start.add(Calendar.MONTH, 1);
-        }
-        while (months < 0) {
-            months += 12;
-            years -= 1;
-            if (start instanceof GregorianCalendar) {
-                if ( ((GregorianCalendar) start).isLeapYear(start.get(Calendar.YEAR) + 1) &&
-                     ( end.get(Calendar.MONTH) > 1) )  
-                {
-                    leapDays += 1;
-                }
+       
+        if (Token.containsTokenWithValue(tokens, M)) {
+            while (days < 0) {
+                days += start.getActualMaximum(Calendar.DAY_OF_MONTH);
+                months -= 1;
+                start.add(Calendar.MONTH, 1);
             }
-            if (end instanceof GregorianCalendar) {
-                if ( ((GregorianCalendar) end).isLeapYear(end.get(Calendar.YEAR)) &&
-                     ( end.get(Calendar.MONTH) < 1) )  
-                {
-                    leapDays -= 1;
+
+            while (months < 0) {
+                months += 12;
+                years -= 1;
+            }
+
+            if (!Token.containsTokenWithValue(tokens, y) && years != 0) {
+                while (years != 0) {
+                    months += 12 * years;
+                    years = 0;
                 }
             }
-            start.add(Calendar.YEAR, 1);
-        }
+        } else {
+            // there are no M's in the format string
 
-        // This rest of this code adds in values that 
-        // aren't requested. This allows the user to ask for the 
-        // number of months and get the real count and not just 0->11.
-        
-        if (!Token.containsTokenWithValue(tokens, y) && years != 0) {
-            if (Token.containsTokenWithValue(tokens, M)) {
-                months += 12 * years;
-                years = 0;
-            } else {
-                while ( (start.get(Calendar.YEAR) != end.get(Calendar.YEAR))) {
-                    days += start.getActualMaximum(Calendar.DAY_OF_YEAR);
+            if( !Token.containsTokenWithValue(tokens, y) ) {
+                int target = end.get(Calendar.YEAR);
+                if (months < 0) {
+                    // target is end-year -1
+                    target -= 1;
+                }
+                
+                while ( (start.get(Calendar.YEAR) != target)) {
+                    days += start.getActualMaximum(Calendar.DAY_OF_YEAR) - start.get(Calendar.DAY_OF_YEAR);
+                    
+                    // Not sure I grok why this is needed, but the brutal tests show it is
+                    if(start instanceof GregorianCalendar) {
+                        if( (start.get(Calendar.MONTH) == Calendar.FEBRUARY) &&
+                            (start.get(Calendar.DAY_OF_MONTH) == 29 ) )
+                        {
+                            days += 1;
+                        }
+                    }
+                    
                     start.add(Calendar.YEAR, 1);
+                    
+                    days += start.get(Calendar.DAY_OF_YEAR);
                 }
+                
                 years = 0;
             }
-        }
-        start.set(Calendar.YEAR, end.get(Calendar.YEAR));
-                
-        if (!Token.containsTokenWithValue(tokens, M) && months != 0) {   
-            while(start.get(Calendar.MONTH) != end.get(Calendar.MONTH)) {
-                String date = start.getTime().toString();
+            
+            while( start.get(Calendar.MONTH) != end.get(Calendar.MONTH) ) {
                 days += start.getActualMaximum(Calendar.DAY_OF_MONTH);
                 start.add(Calendar.MONTH, 1);
             }
-            days += leapDays;
+            
             months = 0;            
+
+            while (days < 0) {
+                days += start.getActualMaximum(Calendar.DAY_OF_MONTH);
+                months -= 1;
+                start.add(Calendar.MONTH, 1);
+            }
+            
         }
-        start.set(Calendar.MONTH, end.get(Calendar.MONTH));
+
+        // The rest of this code adds in values that 
+        // aren't requested. This allows the user to ask for the 
+        // number of months and get the real count and not just 0->11.
 
         if (!Token.containsTokenWithValue(tokens, d)) {
             hours += 24 * days;

Modified: jakarta/commons/proper/lang/trunk/src/test/org/apache/commons/lang/time/DurationFormatUtilsTest.java
URL: http://svn.apache.org/viewvc/jakarta/commons/proper/lang/trunk/src/test/org/apache/commons/lang/time/DurationFormatUtilsTest.java?view=diff&rev=488926&r1=488925&r2=488926
==============================================================================
--- jakarta/commons/proper/lang/trunk/src/test/org/apache/commons/lang/time/DurationFormatUtilsTest.java (original)
+++ jakarta/commons/proper/lang/trunk/src/test/org/apache/commons/lang/time/DurationFormatUtilsTest.java Tue Dec 19 22:10:26 2006
@@ -469,31 +469,77 @@
 
         assertEqualDuration( "365", new int[] { 2007, 2, 2, 0, 0, 0 },
                 new int[] { 2008, 2, 1, 0, 0, 0 }, "dd"); 
-  //      assertEqualDuration( "333", new int[] { 2007, 1, 2, 0, 0, 0 },
-  //              new int[] { 2008, 0, 1, 0, 0, 0 }, "dd"); 
+        assertEqualDuration( "333", new int[] { 2007, 1, 2, 0, 0, 0 },
+                new int[] { 2008, 0, 1, 0, 0, 0 }, "dd"); 
+
+        assertEqualDuration( "28", new int[] { 2008, 1, 2, 0, 0, 0 },
+                new int[] { 2008, 2, 1, 0, 0, 0 }, "dd"); 
+        assertEqualDuration( "393", new int[] { 2007, 1, 2, 0, 0, 0 },
+                new int[] { 2008, 2, 1, 0, 0, 0 }, "dd"); 
+
+        assertEqualDuration( "369", new int[] { 2004, 0, 29, 0, 0, 0 },
+                new int[] { 2005, 1, 1, 0, 0, 0 }, "dd"); 
+
+        assertEqualDuration( "338", new int[] { 2004, 1, 29, 0, 0, 0 },
+                new int[] { 2005, 1, 1, 0, 0, 0 }, "dd"); 
+
+        assertEqualDuration( "28", new int[] { 2004, 2, 8, 0, 0, 0 },
+                new int[] { 2004, 3, 5, 0, 0, 0 }, "dd"); 
+
+        assertEqualDuration( "48", new int[] { 1992, 1, 29, 0, 0, 0 },
+                new int[] { 1996, 1, 29, 0, 0, 0 }, "M"); 
+        
+        
+        // this seems odd - and will fail if I throw it in as a brute force 
+        // below as it expects the answer to be 12. It's a tricky edge case
+        assertEqualDuration( "11", new int[] { 1996, 1, 29, 0, 0, 0 },
+                new int[] { 1997, 1, 28, 0, 0, 0 }, "M"); 
+        // again - this seems odd
+        assertEqualDuration( "11 28", new int[] { 1996, 1, 29, 0, 0, 0 },
+                new int[] { 1997, 1, 28, 0, 0, 0 }, "M d"); 
+        
     }
     
     public void testDurationsByBruteForce() {
-        bruteForce(2006, 0, 1);
-        bruteForce(2006, 0, 2);
-  //      bruteForce(2007, 1, 2);
+        bruteForce(2006, 0, 1, "d", Calendar.DAY_OF_MONTH);
+        bruteForce(2006, 0, 2, "d", Calendar.DAY_OF_MONTH);
+        bruteForce(2007, 1, 2, "d", Calendar.DAY_OF_MONTH);
+        bruteForce(2004, 1, 29, "d", Calendar.DAY_OF_MONTH);
+        bruteForce(1996, 1, 29, "d", Calendar.DAY_OF_MONTH);
+
+        bruteForce(1969, 1, 28, "M", Calendar.MONTH);  // tests for 48 years
+        //bruteForce(1996, 1, 29, "M", Calendar.MONTH);  // this will fail
     }
-        
-    private void bruteForce(int year, int month, int day) {
+    
+    private int FOUR_YEARS = 365 * 3 + 366;
+    
+    // Takes a minute to run, so generally turned off
+//    public void testBrutally() {
+//        Calendar c = Calendar.getInstance();
+//        c.set(2004, 0, 1, 0, 0, 0);
+//        for (int i=0; i < FOUR_YEARS; i++) {
+//            bruteForce(c.get(Calendar.YEAR), c.get(Calendar.MONTH), c.get(Calendar.DAY_OF_MONTH), "d", Calendar.DAY_OF_MONTH );
+//            c.add(Calendar.DAY_OF_MONTH, 1);
+//        }
+//    }        
+    
+    private void bruteForce(int year, int month, int day, String format, int calendarType) {
         String msg = year + "-" + month + "-" + day + " to ";
         Calendar c = Calendar.getInstance();
         c.set(year, month, day, 0, 0, 0);
         int[] array1 = new int[] { year, month, day, 0, 0, 0 };
         int[] array2 = new int[] { year, month, day, 0, 0, 0 };
-        for (int i=0; i < 1500; i++) {
+        for (int i=0; i < FOUR_YEARS; i++) {
             array2[0] = c.get(Calendar.YEAR);
             array2[1] = c.get(Calendar.MONTH);
             array2[2] = c.get(Calendar.DAY_OF_MONTH);
             String tmpMsg = msg + array2[0] + "-" + array2[1] + "-" + array2[2] + " at ";
-            assertEqualDuration( tmpMsg + i, Integer.toString(i), array1, array2, "d" );
-            c.add(Calendar.DAY_OF_MONTH, 1);
+            assertEqualDuration( tmpMsg + i, Integer.toString(i), array1, array2, format );
+            c.add(calendarType, 1);
         }
     }
+    
+    
 
     private void assertEqualDuration(String expected, int[] start, int[] end, String format) {
         assertEqualDuration(null, expected, start, end, format);



---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org