Refactored design and implementation smells

src/main/java/com/cronutils/builder/CronBuilder.java

@@ -21,6 +21,7 @@
 import com.cronutils.model.field.CronFieldName;
 import com.cronutils.model.field.constraint.FieldConstraints;
 import com.cronutils.model.field.definition.FieldDefinition;
+import com.cronutils.model.field.expression.Always;
 import com.cronutils.model.field.expression.FieldExpression;
 import com.cronutils.model.field.expression.On;
 import com.cronutils.model.field.expression.visitor.ValidationFieldExpressionVisitor;
@@ -106,7 +107,7 @@ public static Cron yearly(final CronDefinition definition){
             builder = builder.withMonth(new On(new IntegerFieldValue(1)));
         }
         if(definition.containsFieldDefinition(DAY_OF_WEEK)){
-            builder = builder.withDoW(FieldExpression.always());
+            builder = builder.withDoW(Always.always());
         }
         return builder.instance();
     }
@@ -130,10 +131,10 @@ public static Cron monthly(final CronDefinition definition){
             builder = builder.withDoM(new On(new IntegerFieldValue(1)));
         }
         if(definition.containsFieldDefinition(MONTH)){
-            builder = builder.withMonth(FieldExpression.always());
+            builder = builder.withMonth(Always.always());
         }
         if(definition.containsFieldDefinition(DAY_OF_WEEK)){
-            builder = builder.withDoW(FieldExpression.always());
+            builder = builder.withDoW(Always.always());
         }
         return builder.instance();
     }
@@ -150,10 +151,10 @@ public static Cron weekly(final CronDefinition definition){
             builder = builder.withHour(new On(new IntegerFieldValue(0)));
         }
         if(definition.containsFieldDefinition(DAY_OF_MONTH)){
-            builder = builder.withDoM(FieldExpression.always());
+            builder = builder.withDoM(Always.always());
         }
         if(definition.containsFieldDefinition(MONTH)){
-            builder = builder.withMonth(FieldExpression.always());
+            builder = builder.withMonth(Always.always());
         }
         if(definition.containsFieldDefinition(DAY_OF_WEEK)){
             builder = builder.withDoW(new On(new IntegerFieldValue(0)));
@@ -173,13 +174,13 @@ public static Cron daily(final CronDefinition definition){
             builder = builder.withHour(new On(new IntegerFieldValue(0)));
         }
         if(definition.containsFieldDefinition(DAY_OF_MONTH)){
-            builder = builder.withDoM(FieldExpression.always());
+            builder = builder.withDoM(Always.always());
         }
         if(definition.containsFieldDefinition(MONTH)){
-            builder = builder.withMonth(FieldExpression.always());
+            builder = builder.withMonth(Always.always());
         }
         if(definition.containsFieldDefinition(DAY_OF_WEEK)){
-            builder = builder.withDoW(FieldExpression.always());
+            builder = builder.withDoW(Always.always());
         }
         return builder.instance();
     }
@@ -197,16 +198,16 @@ public static Cron hourly(final CronDefinition definition){
             builder = builder.withMinute(new On(new IntegerFieldValue(0)));
         }
         if(definition.containsFieldDefinition(HOUR)){
-            builder = builder.withHour(FieldExpression.always());
+            builder = builder.withHour(Always.always());
         }
         if(definition.containsFieldDefinition(DAY_OF_MONTH)){
-            builder = builder.withDoM(FieldExpression.always());
+            builder = builder.withDoM(Always.always());
         }
         if(definition.containsFieldDefinition(MONTH)){
-            builder = builder.withMonth(FieldExpression.always());
+            builder = builder.withMonth(Always.always());
         }
         if(definition.containsFieldDefinition(DAY_OF_WEEK)){
-            builder = builder.withDoW(FieldExpression.always());
+            builder = builder.withDoW(Always.always());
         }
         return builder.instance();
     }

src/main/java/com/cronutils/descriptor/NominalDescriptionStrategy.java

@@ -20,7 +20,7 @@
 import java.util.ResourceBundle;
 import java.util.Set;
 
-import static com.cronutils.model.field.expression.FieldExpression.always;
+import static com.cronutils.model.field.expression.Always.always;
 
 /**
  * Description strategy where a cron field number can be mapped to a name.

src/main/java/com/cronutils/descriptor/TimeDescriptionStrategy.java

@@ -23,7 +23,7 @@
 import java.util.ResourceBundle;
 import java.util.Set;
 
-import static com.cronutils.model.field.expression.FieldExpression.always;
+import static com.cronutils.model.field.expression.Always.always;
 
 /**
  * Strategy to provide a human readable description to hh:mm:ss variations.

src/main/java/com/cronutils/mapper/CronMapper.java

@@ -41,8 +41,8 @@
 import java.util.List;
 import java.util.Map;
 
-import static com.cronutils.model.field.expression.FieldExpression.always;
-import static com.cronutils.model.field.expression.FieldExpression.questionMark;
+import static com.cronutils.model.field.expression.Always.always;
+import static com.cronutils.model.field.expression.QuestionMark.questionMark;
 
 public class CronMapper {
     private final Map<CronFieldName, Function<CronField, CronField>> mappings;

src/main/java/com/cronutils/model/CompositeCron.java

@@ -21,7 +21,8 @@ public CompositeCron(List<Cron> crons){
         this.crons = Collections.unmodifiableList(crons);
         Preconditions.checkNotNullNorEmpty(crons, "List of Cron cannot be null or empty");
         this.definition = crons.get(0).getCronDefinition();
-        Preconditions.checkArgument(crons.size()==crons.stream().filter(c->c.getCronDefinition().equals(definition)).count(), "All Cron objects must have same definition for CompositeCron");
+        long sameDefCronsCount = crons.stream().filter(c->c.getCronDefinition().equals(definition)).count();
+        Preconditions.checkArgument(crons.size()==sameDefCronsCount, "All Cron objects must have same definition for CompositeCron");
     }
 
     public List<Cron> getCrons() {

src/main/java/com/cronutils/model/field/constraint/FieldConstraints.java

@@ -13,8 +13,11 @@
 
 package com.cronutils.model.field.constraint;
 
+import com.cronutils.model.field.value.FieldValue;
+import com.cronutils.model.field.value.IntegerFieldValue;
 import com.cronutils.model.field.value.SpecialChar;
 import com.cronutils.utils.Preconditions;
+import com.cronutils.utils.VisibleForTesting;
 
 import java.io.Serializable;
 import java.util.Collections;
@@ -66,6 +69,22 @@ public int getEndRange() {
     public Set<SpecialChar> getSpecialChars() {
         return specialChars;
     }
+
+    /**
+     * Check if given number is greater or equal to start range and minor or equal to end range.
+     *
+     * @param fieldValue - to be validated
+     * @throws IllegalArgumentException - if not in range
+     */
+    @VisibleForTesting
+    public void isInRange(final FieldValue<?> fieldValue) {
+        if (fieldValue instanceof IntegerFieldValue) {
+            final int value = ((IntegerFieldValue) fieldValue).getValue();
+            if (!isInRange(value)) {
+                throw new IllegalArgumentException(String.format("Value %s not in range [%s, %s]", value, getStartRange(), getEndRange()));
+            }
+        }
+    }
 
     /**
      * Check if given number is greater or equal to start range and minor or equal to end range.
@@ -76,6 +95,23 @@ public boolean isInRange(final int value) {
         return value >= getStartRange() && value <= getEndRange();
     }
 
+    /**
+     * Check if given period is compatible with range.
+     *
+     * @param fieldValue - to be validated
+     * @throws IllegalArgumentException - if not in range
+     */
+    @VisibleForTesting
+    public void isPeriodInRange(final FieldValue<?> fieldValue) {
+        if (fieldValue instanceof IntegerFieldValue) {
+            final int value = ((IntegerFieldValue) fieldValue).getValue();
+            if (!isPeriodInRange(value)) {
+                throw new IllegalArgumentException(
+                        String.format("Period %s not in range [%s, %s]", value, getStartRange(), getEndRange()));
+            }
+        }
+    }
+
     /**
      * Check if given period is compatible with the given range.
      *

src/main/java/com/cronutils/model/field/expression/Always.java

@@ -45,4 +45,8 @@ public String asString() {
     public String toString() {
         return "Always{}";
     }
+
+    public static FieldExpression always() {
+		return INSTANCE;
+	}
 }

src/main/java/com/cronutils/model/field/expression/Every.java

@@ -29,7 +29,7 @@ public class Every extends FieldExpression {
     private final IntegerFieldValue period;
 
     public Every(final IntegerFieldValue time) {
-        this(always(), time);
+        this(Always.always(), time);
     }
 
     public Every(final FieldExpression expression, final IntegerFieldValue period) {

src/main/java/com/cronutils/model/field/expression/FieldExpression.java

@@ -42,12 +42,4 @@ public And and(final FieldExpression exp) {
 	 * @return FieldExpression copied instance with visitor action performed.
 	 */
 	public abstract FieldExpression accept(final FieldExpressionVisitor visitor);
-
-	public static FieldExpression always() {
-		return Always.INSTANCE;
-	}
-
-	public static FieldExpression questionMark() {
-		return QuestionMark.INSTANCE;
-	}
 }

src/main/java/com/cronutils/model/field/expression/QuestionMark.java

@@ -43,4 +43,8 @@ public String asString() {
     public String toString() {
         return "QuestionMark{}";
     }
+
+    public static FieldExpression questionMark() {
+		return INSTANCE;
+	}
 }

src/main/java/com/cronutils/model/field/expression/visitor/ValidationFieldExpressionVisitor.java

@@ -23,8 +23,6 @@
 import com.cronutils.utils.VisibleForTesting;
 
 public class ValidationFieldExpressionVisitor implements FieldExpressionVisitor {
-    private static final String OORANGE = "Value %s not in range [%s, %s]";
-
     private final FieldConstraints constraints;
     private final StringValidations stringValidations;
 
@@ -68,7 +66,7 @@ public Between visit(final Between between) {
         this.checkUnsupportedChars(between);
         preConditions(between);
 
-        if ((constraints.isStrictRange()) && between.getFrom() instanceof IntegerFieldValue && between.getTo() instanceof IntegerFieldValue) {
+        if (isIntegerStrictRange(between)) {
             final int from = ((IntegerFieldValue) between.getFrom()).getValue();
             final int to = ((IntegerFieldValue) between.getTo()).getValue();
             if (from > to) {
@@ -84,18 +82,18 @@ public Every visit(final Every every) {
         this.checkUnsupportedChars(every);
         if (every.getExpression() != null)
             every.getExpression().accept(this);
-        isPeriodInRange(every.getPeriod());
+        constraints.isPeriodInRange(every.getPeriod());
         return every;
     }
 
     @Override
     public On visit(final On on) {
         this.checkUnsupportedChars(on);
         if (!isDefault(on.getTime())) {
-            isInRange(on.getTime());
+        	constraints.isInRange(on.getTime());
         }
         if (!isDefault(on.getNth())) {
-            isInRange(on.getNth());
+        	constraints.isInRange(on.getNth());
         }
         return on;
     }
@@ -107,46 +105,13 @@ public QuestionMark visit(final QuestionMark questionMark) {
     }
 
     private void preConditions(final Between between) {
-        isInRange(between.getFrom());
-        isInRange(between.getTo());
+    	constraints.isInRange(between.getFrom());
+    	constraints.isInRange(between.getTo());
         if (isSpecialCharNotL(between.getFrom()) || isSpecialCharNotL(between.getTo())) {
             throw new IllegalArgumentException("No special characters allowed in range, except for 'L'");
         }
     }
 
-    /**
-     * Check if given number is greater or equal to start range and minor or equal to end range.
-     *
-     * @param fieldValue - to be validated
-     * @throws IllegalArgumentException - if not in range
-     */
-    @VisibleForTesting
-    protected void isInRange(final FieldValue<?> fieldValue) {
-        if (fieldValue instanceof IntegerFieldValue) {
-            final int value = ((IntegerFieldValue) fieldValue).getValue();
-            if (!constraints.isInRange(value)) {
-                throw new IllegalArgumentException(String.format(OORANGE, value, constraints.getStartRange(), constraints.getEndRange()));
-            }
-        }
-    }
-
-    /**
-     * Check if given period is compatible with range.
-     *
-     * @param fieldValue - to be validated
-     * @throws IllegalArgumentException - if not in range
-     */
-    @VisibleForTesting
-    protected void isPeriodInRange(final FieldValue<?> fieldValue) {
-        if (fieldValue instanceof IntegerFieldValue) {
-            final int value = ((IntegerFieldValue) fieldValue).getValue();
-            if (!constraints.isPeriodInRange(value)) {
-                throw new IllegalArgumentException(
-                        String.format("Period %s not in range [%s, %s]", value, constraints.getStartRange(), constraints.getEndRange()));
-            }
-        }
-    }
-
     @VisibleForTesting
     protected boolean isDefault(final FieldValue<?> fieldValue) {
         return fieldValue instanceof IntegerFieldValue && ((IntegerFieldValue) fieldValue).getValue() == -1;
@@ -155,4 +120,10 @@ protected boolean isDefault(final FieldValue<?> fieldValue) {
     protected boolean isSpecialCharNotL(final FieldValue<?> fieldValue) {
         return fieldValue instanceof SpecialCharFieldValue && !SpecialChar.L.equals(fieldValue.getValue());
     }
+
+    private boolean isIntegerStrictRange(final Between between) {
+    	return constraints.isStrictRange() 
+    			&& between.getFrom() instanceof IntegerFieldValue 
+    			&& between.getTo() instanceof IntegerFieldValue;
+    }
 }

src/main/java/com/cronutils/model/field/expression/visitor/ValueMappingFieldExpressionVisitor.java

@@ -18,7 +18,7 @@
 import com.cronutils.model.field.value.FieldValue;
 import com.cronutils.model.field.value.IntegerFieldValue;
 
-import static com.cronutils.model.field.expression.FieldExpression.questionMark;
+import static com.cronutils.model.field.expression.QuestionMark.questionMark;
 
 /**
  * Performs a transformation on FieldExpression values.