runtime-field: Fix #361 Make all comparators immutable

speedment · Mar 3, 2017 · 20772f3 · 20772f3
1 parent 1ebf706
commit 20772f3
Show file tree

Hide file tree

Showing 9 changed files with 113 additions and 76 deletions.
diff --git a/...t/runtime-field/src/main/java/com/speedment/runtime/field/comparator/FieldComparator.java b/...t/runtime-field/src/main/java/com/speedment/runtime/field/comparator/FieldComparator.java
@@ -22,6 +22,10 @@
 /**
  * A specialized {@link Comparator} that contains meta data information about 
  * the field that is being compared.
+ * <p>
+ * Implementations of this interface are immutable. Equality is based upon the
+ * equality of the {@link #getField() field}-identifier, the 
+ * {@link #getNullOrder() null strategy} and the {@link #isReversed() order}.
  * 
  * @param <ENTITY>  the entity type
  * @param <V>       the value type
@@ -48,9 +52,6 @@ public interface FieldComparator<ENTITY, V extends Comparable<? super V>>
      * @return  the null order strategy
      */
     NullOrder getNullOrder();
-
-    @Override
-    FieldComparator<ENTITY, V> reversed();
 
     /**
      * Returns {@code true} if this comparator reverses the natural order of the
@@ -61,4 +62,15 @@ public interface FieldComparator<ENTITY, V extends Comparable<? super V>>
      */
     boolean isReversed();
 
+    /**
+     * Returns a new {@code FieldComparator} that order entities in the opposite
+     * orders compared to this comparator. For an example, if this comparator
+     * orders entities based on a column 'firstname' in descending order, then
+     * the returned {@code FieldComparator} will be ordering entities based on
+     * 'firstname' in ascending order.
+     * 
+     * @return  a new reverse comparator
+     */
+    @Override
+    FieldComparator<ENTITY, V> reversed();
 }
diff --git a/...rc/main/java/com/speedment/runtime/field/internal/comparator/ByteFieldComparatorImpl.java b/...rc/main/java/com/speedment/runtime/field/internal/comparator/ByteFieldComparatorImpl.java
@@ -35,11 +35,15 @@
 public final class ByteFieldComparatorImpl<ENTITY, D> implements ByteFieldComparator<ENTITY, D> {
 
     private final ByteField<ENTITY, D> field;
-    private boolean reversed;
+    private final boolean reversed;
 
     public ByteFieldComparatorImpl(ByteField<ENTITY, D> field) {
+        this(field, false);
+    }
+
+    public ByteFieldComparatorImpl(ByteField<ENTITY, D> field, boolean reversed) {
         this.field    = requireNonNull(field);
-        this.reversed = false;
+        this.reversed = reversed;
     }
 
     @Override
@@ -59,8 +63,7 @@ public boolean isReversed() {
 
     @Override
     public FieldComparator<ENTITY, Byte> reversed() {
-        reversed = !reversed;
+        return new ByteFieldComparatorImpl<>(field, !reversed);
-        return this;
     }
 
     @Override

diff --git a/...rc/main/java/com/speedment/runtime/field/internal/comparator/CharFieldComparatorImpl.java b/...rc/main/java/com/speedment/runtime/field/internal/comparator/CharFieldComparatorImpl.java
@@ -35,11 +35,15 @@
 public final class CharFieldComparatorImpl<ENTITY, D> implements CharFieldComparator<ENTITY, D> {
 
     private final CharField<ENTITY, D> field;
-    private boolean reversed;
+    private final boolean reversed;
 
     public CharFieldComparatorImpl(CharField<ENTITY, D> field) {
+        this(field, false);
+    }
+
+    public CharFieldComparatorImpl(CharField<ENTITY, D> field, boolean reversed) {
         this.field    = requireNonNull(field);
-        this.reversed = false;
+        this.reversed = reversed;
     }
 
     @Override
@@ -59,8 +63,7 @@ public boolean isReversed() {
 
     @Override
     public FieldComparator<ENTITY, Character> reversed() {
-        reversed = !reversed;
+        return new CharFieldComparatorImpl<>(field, !reversed);
-        return this;
     }
 
     @Override

diff --git a/.../main/java/com/speedment/runtime/field/internal/comparator/DoubleFieldComparatorImpl.java b/.../main/java/com/speedment/runtime/field/internal/comparator/DoubleFieldComparatorImpl.java
@@ -35,11 +35,15 @@
 public final class DoubleFieldComparatorImpl<ENTITY, D> implements DoubleFieldComparator<ENTITY, D> {
 
     private final DoubleField<ENTITY, D> field;
-    private boolean reversed;
+    private final boolean reversed;
 
     public DoubleFieldComparatorImpl(DoubleField<ENTITY, D> field) {
+        this(field, false);
+    }
+
+    public DoubleFieldComparatorImpl(DoubleField<ENTITY, D> field, boolean reversed) {
         this.field    = requireNonNull(field);
-        this.reversed = false;
+        this.reversed = reversed;
     }
 
     @Override
@@ -59,8 +63,7 @@ public boolean isReversed() {
 
     @Override
     public FieldComparator<ENTITY, Double> reversed() {
-        reversed = !reversed;
+        return new DoubleFieldComparatorImpl<>(field, !reversed);
-        return this;
     }
 
     @Override

diff --git a/...c/main/java/com/speedment/runtime/field/internal/comparator/FloatFieldComparatorImpl.java b/...c/main/java/com/speedment/runtime/field/internal/comparator/FloatFieldComparatorImpl.java
@@ -35,11 +35,15 @@
 public final class FloatFieldComparatorImpl<ENTITY, D> implements FloatFieldComparator<ENTITY, D> {
 
     private final FloatField<ENTITY, D> field;
-    private boolean reversed;
+    private final boolean reversed;
 
     public FloatFieldComparatorImpl(FloatField<ENTITY, D> field) {
+        this(field, false);
+    }
+
+    public FloatFieldComparatorImpl(FloatField<ENTITY, D> field, boolean reversed) {
         this.field    = requireNonNull(field);
-        this.reversed = false;
+        this.reversed = reversed;
     }
 
     @Override
@@ -59,8 +63,7 @@ public boolean isReversed() {
 
     @Override
     public FieldComparator<ENTITY, Float> reversed() {
-        reversed = !reversed;
+        return new FloatFieldComparatorImpl<>(field, !reversed);
-        return this;
     }
 
     @Override

diff --git a/...src/main/java/com/speedment/runtime/field/internal/comparator/IntFieldComparatorImpl.java b/...src/main/java/com/speedment/runtime/field/internal/comparator/IntFieldComparatorImpl.java
@@ -35,11 +35,15 @@
 public final class IntFieldComparatorImpl<ENTITY, D> implements IntFieldComparator<ENTITY, D> {
 
     private final IntField<ENTITY, D> field;
-    private boolean reversed;
+    private final boolean reversed;
 
     public IntFieldComparatorImpl(IntField<ENTITY, D> field) {
+        this(field, false);
+    }
+
+    public IntFieldComparatorImpl(IntField<ENTITY, D> field, boolean reversed) {
         this.field    = requireNonNull(field);
-        this.reversed = false;
+        this.reversed = reversed;
     }
 
     @Override
@@ -59,8 +63,7 @@ public boolean isReversed() {
 
     @Override
     public FieldComparator<ENTITY, Integer> reversed() {
-        reversed = !reversed;
+        return new IntFieldComparatorImpl<>(field, !reversed);
-        return this;
     }
 
     @Override

diff --git a/...rc/main/java/com/speedment/runtime/field/internal/comparator/LongFieldComparatorImpl.java b/...rc/main/java/com/speedment/runtime/field/internal/comparator/LongFieldComparatorImpl.java
@@ -35,11 +35,15 @@
 public final class LongFieldComparatorImpl<ENTITY, D> implements LongFieldComparator<ENTITY, D> {
 
     private final LongField<ENTITY, D> field;
-    private boolean reversed;
+    private final boolean reversed;
 
     public LongFieldComparatorImpl(LongField<ENTITY, D> field) {
+        this(field, false);
+    }
+
+    public LongFieldComparatorImpl(LongField<ENTITY, D> field, boolean reversed) {
         this.field    = requireNonNull(field);
-        this.reversed = false;
+        this.reversed = reversed;
     }
 
     @Override
@@ -59,8 +63,7 @@ public boolean isReversed() {
 
     @Override
     public FieldComparator<ENTITY, Long> reversed() {
-        reversed = !reversed;
+        return new LongFieldComparatorImpl<>(field, !reversed);
-        return this;
     }
 
     @Override

diff --git a/...in/java/com/speedment/runtime/field/internal/comparator/ReferenceFieldComparatorImpl.java b/...in/java/com/speedment/runtime/field/internal/comparator/ReferenceFieldComparatorImpl.java
@@ -30,25 +30,38 @@
  * @param <V>       the value type
  * 
  * @author  Per Minborg
+ * @author  Emil Forslund
  * @since   2.2.0
  */
 public final class ReferenceFieldComparatorImpl
     <ENTITY, D, V extends Comparable<? super V>> 
 implements ReferenceFieldComparator<ENTITY, D, V> {
 
-    private final ComparableField<ENTITY, D, V> referenceField;
+    private final ComparableField<ENTITY, D, V> field;
     private final NullOrder nullOrder;
+    private final boolean reversed;
 
-    private boolean reversed;
+    public ReferenceFieldComparatorImpl(
+            ComparableField<ENTITY, D, V> field, 
+            NullOrder nullOrder) {
+
+        this(field, nullOrder, false);
+    }
+
 
-    public ReferenceFieldComparatorImpl(ComparableField<ENTITY, D, V> referenceField, NullOrder nullOrder) {
+    public ReferenceFieldComparatorImpl(
-        this.referenceField = referenceField;
+            ComparableField<ENTITY, D, V> field, 
-        this.nullOrder = nullOrder;
+            NullOrder nullOrder,
+            boolean reversed) {
+
+        this.field     = requireNonNull(field);
+        this.nullOrder = requireNonNull(nullOrder);
+        this.reversed  = reversed;
     }
 
     @Override
     public ComparableField<ENTITY, D, V> getField() {
-        return referenceField;
+        return field;
     }
 
     @Override
@@ -63,34 +76,36 @@ public boolean isReversed() {
 
     @Override
     public FieldComparator<ENTITY, V> reversed() {
-        reversed = !reversed;
+        return new ReferenceFieldComparatorImpl<>(field, nullOrder, reversed);
-        return this;
     }
 
-    // TODO: Imrpove performance of chained comparators.
+    // TODO: Improve performance of chained comparators.
 
     @Override
     public int compare(ENTITY o1, ENTITY o2) {
-        final V o1Value = referenceField.get(requireNonNull(o1));
+        final V o1Value = field.get(requireNonNull(o1));
-        final V o2Value = referenceField.get(requireNonNull(o2));
+        final V o2Value = field.get(requireNonNull(o2));
+
         if (o1Value == null && o2Value == null) {
             if (NullOrder.NONE == nullOrder) {
-                throw new NullPointerException("Both fields were null and null fields not allowed");
+                throw new NullPointerException(
+                    "Both fields were null and null fields not allowed"
+                );
             }
             return 0;
-        }
+        } else if (o1Value == null) {
-        if (o1Value == null) {
             return forNull(Parameter.FIRST);
-        }
+        } else if (o2Value == null) {
-        if (o2Value == null) {
             return forNull(Parameter.SECOND);
         }
+
         return applyReversed(o1Value.compareTo(o2Value));
     }
 
     @Override
     public int hashCode() {
-        return (4049 + Objects.hashCode(referenceField.identifier())) * 3109
+        return ((4049 + Objects.hashCode(field.identifier())) * 3109
+            + nullOrder.hashCode()) * 1039
             + Boolean.hashCode(reversed);
     }
 
@@ -104,50 +119,39 @@ public boolean equals(Object obj) {
         final FieldComparator<ENTITY, V> casted =
             (FieldComparator<ENTITY, V>) obj;
 
-        return reversed == casted.isReversed()
+        return reversed  == casted.isReversed()
+            && nullOrder == casted.getNullOrder()
             && Objects.equals(
-                referenceField.identifier(),
+                field.identifier(),
                 casted.getField().identifier()
             );
     }
 
     @Override
     public String toString() {
-        return "(order by " + referenceField.identifier() + " " +
+        return "(order by " + field.identifier() + " " +
             (reversed ? "descending" : "ascending") + ")";
     }
 
-    private enum Parameter {
-        FIRST, SECOND
-    }
-
     private int forNull(Parameter parameter) {
         final int firstOutcome = Parameter.FIRST.equals(parameter) ? -1 : 1;
-        final int lastOutcome = -firstOutcome;
+        final int lastOutcome  = -firstOutcome;
+
         switch (nullOrder) {
-            case NONE:
+            case FIRST : return applyReversed(firstOutcome);
-                throw new NullPointerException(
+            case LAST  : return applyReversed(lastOutcome);
-                    "A field was null and null fields not allowed"
+            case NONE  : throw new NullPointerException(
-                );
+                "A field was null and null fields not allowed"
-            case FIRST:
+            );
-                return applyReversed(firstOutcome);
+            default : throw new IllegalStateException("Illegal NullOrder");
-            case LAST:
-                return applyReversed(lastOutcome);
-            default:
-                throw new IllegalStateException("Illegal NullOrder");
         }
     }
 
     private int applyReversed(int compare) {
-        if (!reversed) {
+        return reversed ? -compare : compare;
-            return compare;
+    }
-        }
+
-        if (compare == 0) {
+    private enum Parameter {
-            return 0;
+        FIRST, SECOND
-        } else if (compare > 0) {
-            return -1;
-        } else {
-            return 1;
-        }
     }
 }
diff --git a/...c/main/java/com/speedment/runtime/field/internal/comparator/ShortFieldComparatorImpl.java b/...c/main/java/com/speedment/runtime/field/internal/comparator/ShortFieldComparatorImpl.java
@@ -35,11 +35,15 @@
 public final class ShortFieldComparatorImpl<ENTITY, D> implements ShortFieldComparator<ENTITY, D> {
 
     private final ShortField<ENTITY, D> field;
-    private boolean reversed;
+    private final boolean reversed;
 
     public ShortFieldComparatorImpl(ShortField<ENTITY, D> field) {
+        this(field, false);
+    }
+
+    public ShortFieldComparatorImpl(ShortField<ENTITY, D> field, boolean reversed) {
         this.field    = requireNonNull(field);
-        this.reversed = false;
+        this.reversed = reversed;
     }
 
     @Override
@@ -59,8 +63,7 @@ public boolean isReversed() {
 
     @Override
     public FieldComparator<ENTITY, Short> reversed() {
-        reversed = !reversed;
+        return new ShortFieldComparatorImpl<>(field, !reversed);
-        return this;
     }
 
     @Override