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: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to