Issue #3: Improving descriptions

jmrozanec · Jan 31, 2015 · ad5c907 · ad5c907
1 parent c1777d3
commit ad5c907
Show file tree

Hide file tree

Showing 6 changed files with 101 additions and 18 deletions.
diff --git a/src/main/java/com/cronutils/descriptor/CronDescriptor.java b/src/main/java/com/cronutils/descriptor/CronDescriptor.java
@@ -56,13 +56,13 @@ public String describe(Cron cron) {
         Validate.notNull(cron, "Cron must not be null");
         Map<CronFieldName, CronField> expressions = cron.retrieveFieldsAsMap();
         return
-                new StringBuilder().append(describeHHmmss(expressions)).append(" ")
+                new StringBuilder()
+                        .append(describeHHmmss(expressions)).append(" ")
                         .append(describeDayOfMonth(expressions)).append(" ")
                         .append(describeMonth(expressions)).append(" ")
                         .append(describeDayOfWeek(expressions)).append(" ")
                         .append(describeYear(expressions))
                         .toString().replaceAll("\\s+", " ").trim();
-
     }
 
     /**

diff --git a/src/main/java/com/cronutils/descriptor/DescriptionStrategy.java b/src/main/java/com/cronutils/descriptor/DescriptionStrategy.java
@@ -124,10 +124,10 @@ protected String describe(And and) {
         StringBuilder builder = new StringBuilder();
         if(!onExpressions.isEmpty()) {
             builder.append(bundle.getString("at"));
-            createAndDescription(builder, onExpressions).append(" %s");
+            createAndDescription(builder, onExpressions).append(" %p");
         }
         if(!expressions.isEmpty()){
-            createAndDescription(builder, expressions).append(" %s");
+            createAndDescription(builder, expressions);
         }
 
         return builder.toString();
@@ -177,11 +177,11 @@ protected String describe(Between between, boolean and) {
     protected String describe(Every every, boolean and) {
         String description;
         if (every.getTime() > 1) {
-            description = String.format("%s %s ", bundle.getString("every"), nominalValue(every.getTime()));
+            description = String.format("%s %s ", bundle.getString("every"), nominalValue(every.getTime())) + " %p ";
         } else {
-            description = bundle.getString("every");
+            description = bundle.getString("every")+" %s ";
         }
-        return description + " %s ";
+        return description;
     }
 
     /**

diff --git a/src/main/java/com/cronutils/descriptor/TimeDescriptionStrategy.java b/src/main/java/com/cronutils/descriptor/TimeDescriptionStrategy.java
@@ -89,10 +89,22 @@ public String describe() {
                 return function.apply(fields);
             }
         }
+        String secondsDesc = "";
+        String minutesDesc = "";
+        String hoursDesc = addTimeExpressions(describe(hours), bundle.getString("hour"), bundle.getString("hours"));
+        if(!(seconds instanceof On && isDefault((On)seconds))){
+            secondsDesc = addTimeExpressions(describe(seconds), bundle.getString("second"), bundle.getString("seconds"));
+        }
+        if(!(minutes instanceof On && isDefault((On)minutes))){
+            minutesDesc = addTimeExpressions(describe(minutes), bundle.getString("minute"), bundle.getString("minutes"));
+        }
+        return  String.format("%s %s %s", secondsDesc, minutesDesc, hoursDesc);
+    }
 
-        return describe(seconds).replaceAll("%s", bundle.getString("seconds")) + " " +
-                describe(minutes).replaceAll("%s", bundle.getString("minutes")) + " " +
-                describe(hours).replaceAll("%s", bundle.getString("hours"));
+    private String addTimeExpressions(String description, String singular, String plural){
+        return description
+                .replaceAll("%s", singular)
+                .replaceAll("%p", plural);
     }
 
     /**
@@ -132,6 +144,9 @@ public String apply(TimeFields timeFields) {
                                 timeFields.minutes instanceof On &&
                                 timeFields.seconds instanceof On) {
                                 if (isDefault((On) timeFields.seconds)) {
+                                    if(isDefault((On) timeFields.minutes)){
+                                        return String.format("%s %s ", bundle.getString("every"), bundle.getString("hour"));
+                                    }
                                     return String.format("%s %s %s %s %s", bundle.getString("every"),
                                             bundle.getString("hour"), bundle.getString("at"),
                                             bundle.getString("minute"), ((On) timeFields.minutes).getTime());
@@ -204,8 +219,8 @@ public String apply(TimeFields timeFields) {
                 new Function<TimeFields, String>() {
                     @Override
                     public String apply(TimeFields timeFields) {
-                        if (timeFields.minutes instanceof Between &&
-                                timeFields.hours instanceof On) {
+                        if (timeFields.hours instanceof On &&
+                                timeFields.minutes instanceof Between) {
                             if (timeFields.seconds instanceof On) {
                                 return String.format("%s %s %s %02d:%02d %s %02d:%02d",
                                         bundle.getString("every"),
@@ -237,9 +252,11 @@ public String apply(TimeFields timeFields) {
                         if (timeFields.hours instanceof Always &&
                                 timeFields.minutes instanceof Every &&
                                 timeFields.seconds instanceof On) {
-                            if (isDefault((On) timeFields.seconds)) {
-                                return String.format("%s %s %s ", bundle.getString("every"), ((Every) minutes).getTime(), bundle.getString("minutes"));
+                            if (((Every) timeFields.minutes).getTime()==1 &&
+                                    isDefault((On) timeFields.seconds)) {
+                                return String.format("%s %s", bundle.getString("every"), bundle.getString("minute"));
                             }
+                            return String.format("%s %s %s ", bundle.getString("every"), ((Every) minutes).getTime(), bundle.getString("minutes"));
                         }
                         return "";
                     }
@@ -253,6 +270,10 @@ public String apply(TimeFields timeFields) {
                         if (timeFields.hours instanceof Every &&
                                 timeFields.minutes instanceof On &&
                                 timeFields.seconds instanceof On) {
+                            //every hour
+                            if(((On) timeFields.minutes).getTime()==0 && ((On) timeFields.seconds).getTime()==0){
+                                return String.format("%s %s", bundle.getString("every"), bundle.getString("hour"));
+                            }
                             String result = String.format("%s %s %s %s %s %s ",
                                     bundle.getString("every"), ((Every) hours).getTime(), bundle.getString("hours"),
                                     bundle.getString("at"), bundle.getString("minute"), ((On) minutes).getTime());
@@ -268,6 +289,7 @@ public String apply(TimeFields timeFields) {
                 });
     }
 
+
     /**
      * Contains CronFieldExpression instances for hours, minutes and seconds.
      */

diff --git a/src/main/resources/CronUtilsI18N_en.properties b/src/main/resources/CronUtilsI18N_en.properties
@@ -1,16 +1,19 @@
 between_x_and_y=between {0} and {1}
 every=every
-second=second
 and=and
 at=at
 day=day
+days=days
 hour=hour
 hours=hours
 minute=minute
 minutes=minutes
+second=second
 seconds=seconds
 month=month
+months=months
 year=year
+years=years
 between=between
 of_every_month=of every month
 of_the_month=of the month

diff --git a/src/test/java/com/cronutils/utils/descriptor/CronDescriptorCron4jIntegrationTest.java b/src/test/java/com/cronutils/utils/descriptor/CronDescriptorCron4jIntegrationTest.java
@@ -0,0 +1,42 @@
+package com.cronutils.utils.descriptor;
+
+import com.cronutils.descriptor.CronDescriptor;
+import com.cronutils.model.CronType;
+import com.cronutils.model.definition.CronDefinitionBuilder;
+import com.cronutils.parser.CronParser;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.util.Locale;
+
+import static org.junit.Assert.assertEquals;
+
+public class CronDescriptorCron4jIntegrationTest {
+        private CronDescriptor descriptor;
+        private CronParser parser;
+
+        @Before
+        public void setUp() throws Exception {
+            descriptor = CronDescriptor.instance(Locale.UK);
+            parser = new CronParser(CronDefinitionBuilder.instanceDefinitionFor(CronType.CRON4J));
+        }
+
+        @Test
+        public void testEveryMinuteBetween1100And1110(){
+            assertEquals("every minute between 11:00 and 11:10", descriptor.describe(parser.parse("0-10 11 * * *")));
+        }
+
+        @Test
+        public void testEveryMinute(){
+            assertEquals("every minute", descriptor.describe(parser.parse("* * * * *")));
+            assertEquals("every minute", descriptor.describe(parser.parse("*/1 * * * *")));
+            assertEquals("every minute", descriptor.describe(parser.parse("0/1 * * * ?")));
+        }
+
+    @Test
+    public void testEveryFiveMinutes(){
+        assertEquals("every 5 minutes", descriptor.describe(parser.parse("*/5 * * * *")));
+        assertEquals("every 5 minutes", descriptor.describe(parser.parse("0/5 * * * ?")));
+    }
+
+}
diff --git a/...riptor/CronDescriptorIntegrationTest.java → .../CronDescriptorQuartzIntegrationTest.java b/...riptor/CronDescriptorIntegrationTest.java → .../CronDescriptorQuartzIntegrationTest.java
@@ -11,7 +11,7 @@
 
 import static org.junit.Assert.assertEquals;
 
-public class CronDescriptorIntegrationTest {
+public class CronDescriptorQuartzIntegrationTest {
 
     private CronDescriptor descriptor;
     private CronParser parser;
@@ -29,14 +29,30 @@ public void testCronWithAndHours(){
 
     @Test
     public void testCronAndWithRangeHours(){
-        assertEquals("at 1, 2, 3 and 4 hours and every hours between 6 and 9 hours",
+        assertEquals("at 1, 2, 3 and 4 hours and every hour between 6 and 9",
                 descriptor.describe(parser.parse("* * 1,2,3,4,6-9 * * * *")));
     }
 
     @Test
     public void testCronAndWithRangesAndEveryExpressions(){
-        assertEquals("at 0 seconds every 3 minutes between 2 and 59 at 1, 9 " +
+        assertEquals("every 3 minutes between 2 and 59 at 1, 9 " +
                 "and 22 hours every day between 11 and 26 every month between January and June",
                 descriptor.describe(parser.parse("0 2-59/3 1,9,22 11-26 1-6 ?")));
     }
+
+    @Test
+    public void testEverySecond(){
+        assertEquals("every second", descriptor.describe(parser.parse("* * * * * *")));
+    }
+
+    @Test
+    public void testEvery45Seconds(){
+        assertEquals("every 45 seconds", descriptor.describe(parser.parse("*/45 * * * * *")));
+    }
+
+    @Test
+    public void testEveryHour(){
+        assertEquals("every hour", descriptor.describe(parser.parse("0 0 * * * ?")));
+        assertEquals("every hour", descriptor.describe(parser.parse("0 0 0/1 * * ?")));
+    }
 }