Merge pull request #1 from jlrigau/review-comparator

Review on new feature compareToOnly

Author: Jean-Eudes

src/main/java/de/danielbechler/diff/BeanDiffer.java

@@ -72,18 +72,18 @@ else if (instances.hasBeenRemoved())
 
 	private void compareUsingAppropriateMethod(final Node beanNode, final Instances instances)
 	{
-		if (nodeInspector.isIntrospectible(beanNode))
+		if (nodeInspector.isCompareToOnly(beanNode))
 		{
-			compareUsingIntrospection(beanNode, instances);
-		}
-		else if (nodeInspector.isComparable(beanNode))
-		{
-			compareUsingCompare(beanNode, instances);
+			compareUsingCompareTo(beanNode, instances);
 		}
 		else if (nodeInspector.isEqualsOnly(beanNode))
 		{
 			compareUsingEquals(beanNode, instances);
 		}
+        else if (nodeInspector.isIntrospectible(beanNode))
+        {
+            compareUsingIntrospection(beanNode, instances);
+        }
 	}
 
 	private void compareUsingIntrospection(final Node beanNode, final Instances beanInstances)
@@ -100,6 +100,19 @@ private void compareUsingIntrospection(final Node beanNode, final Instances bean
 		}
 	}
 
+    @SuppressWarnings({"MethodMayBeStatic"})
+    private void compareUsingCompareTo(final Node beanNode, final Instances instances)
+    {
+        if (instances.areEqualByComparison())
+        {
+            beanNode.setState(Node.State.UNTOUCHED);
+        }
+        else
+        {
+            beanNode.setState(Node.State.CHANGED);
+        }
+    }
+
 	@SuppressWarnings({"MethodMayBeStatic"})
 	private void compareUsingEquals(final Node beanNode, final Instances instances)
 	{
@@ -113,18 +126,6 @@ private void compareUsingEquals(final Node beanNode, final Instances instances)
 		}
 	}
 
-	private void compareUsingCompare(final Node beanNode, final Instances instances)
-	{
-		if (instances.areComparable())
-		{
-			beanNode.setState(Node.State.UNTOUCHED);
-		}
-		else
-		{
-			beanNode.setState(Node.State.CHANGED);
-		}
-	}
-
 	@TestOnly
 	void setIntrospector(final Introspector introspector)
 	{

src/main/java/de/danielbechler/diff/Configuration.java

@@ -77,9 +77,8 @@ public enum PrimitiveDefaultValueMode
 	private final Collection<PropertyPath> includedProperties = new HashSet<PropertyPath>(10);
 	private final Collection<PropertyPath> excludedProperties = new HashSet<PropertyPath>(10);
 	private final Collection<PropertyPath> equalsOnlyProperties = new LinkedHashSet<PropertyPath>(10);
+    private final Collection<Class<?>> compareToOnlyTypes = new LinkedHashSet<Class<?>>(10);
 	private final Collection<Class<?>> equalsOnlyTypes = new LinkedHashSet<Class<?>>(10);
-	private final Collection<Class<?>> comparableTypes = new LinkedHashSet<Class<?>>(10);
-	private final Map<Class<?>, Comparator<?>> compareTypes = new HashMap<Class<?>, Comparator<?>>(10);
 	private boolean returnUnchangedNodes = false;
 	private boolean returnIgnoredNodes = false;
 	private boolean returnCircularNodes = true;
@@ -126,24 +125,18 @@ public Configuration withoutProperty(final PropertyPath propertyPath)
 		return this;
 	}
 
+    public Configuration withCompareToOnlyType(final Class<?> type)
+    {
+        this.compareToOnlyTypes.add(type);
+        return this;
+    }
+
 	public Configuration withEqualsOnlyType(final Class<?> type)
 	{
 		this.equalsOnlyTypes.add(type);
 		return this;
 	}
 
-	public Configuration withComparableType(final Class<?> type)
-	{
-		this.comparableTypes.add(type);
-		return this;
-	}
-
-	public Configuration withCompareType(final Class<?> type, Comparator<?> comparator)
-	{
-		this.compareTypes.put(type, comparator);
-		return this;
-	}
-
 	public Configuration withEqualsOnlyProperty(final PropertyPath propertyPath)
 	{
 		this.equalsOnlyProperties.add(propertyPath);
@@ -271,6 +264,23 @@ public boolean isExcluded(final Node node)
 		return false;
 	}
 
+    public boolean isCompareToOnly(final Node node)
+    {
+        final Class<?> propertyType = node.getType();
+        if (propertyType != null)
+        {
+            if (compareToOnlyTypes.contains(propertyType) && Comparable.class.isAssignableFrom(propertyType))
+            {
+                return true;
+            }
+            if (Classes.isComparableType(propertyType))
+            {
+                return true;
+            }
+        }
+        return false;
+    }
+
 	public boolean isEqualsOnly(final Node node)
 	{
 		final Class<?> propertyType = node.getType();
@@ -300,7 +310,6 @@ public boolean isEqualsOnly(final Node node)
 		return false;
 	}
 
-
 	public boolean isReturnable(final Node node)
 	{
 		if (node.isIgnored())
@@ -326,21 +335,9 @@ else if (node.hasChildren())
 		return true;
 	}
 
-    public boolean isComparable(Node node) {
-        return comparableTypes.contains(node.getType()) && Comparable.class.isAssignableFrom(node.getType());
-    }
-
     public boolean isIntrospectible(final Node node)
 	{
-		if (isEqualsOnly(node))
-		{
-			return false;
-		}
-        else if (isComparable(node))
-		{
-			return false;
-		}
-		else if (node.isAdded())
+		if (node.isAdded())
 		{
 			return returnChildrenOfAddedNodes;
 		}

src/main/java/de/danielbechler/diff/Instances.java

@@ -25,6 +25,7 @@
 import java.util.*;
 
 import static de.danielbechler.util.Objects.*;
+import static de.danielbechler.util.Comparables.*;
 
 /** @author Daniel Bechler */
 @SuppressWarnings({"UnusedDeclaration"})
@@ -172,15 +173,11 @@ public boolean areEqual()
 		return isEqual(base, working);
 	}
 
-    public boolean areComparable()
+    public boolean areEqualByComparison()
 	{
-		return areComparable((Comparable) base, (Comparable) working);
+		return isEqualByComparison((Comparable) base, (Comparable) working);
 	}
 
-    private boolean areComparable(Comparable base, Comparable working) {
-        return base.compareTo(working) == 0;
-    }
-
     public boolean areSame()
 	{
 		return working == base;

src/main/java/de/danielbechler/diff/NodeInspector.java

@@ -27,12 +27,12 @@ interface NodeInspector
 
 	boolean isExcluded(Node node);
 
+    boolean isCompareToOnly(Node node);
+
 	boolean isEqualsOnly(Node node);
 
 	boolean isReturnable(Node node);
 
-    boolean isComparable(Node node);
-
 	/**
 	 * @return Returns <code>true</code> if the object represented by the given node should be compared via
 	 *         introspection. It must always return </code><code>false</code> if {@link

src/main/java/de/danielbechler/util/Classes.java

@@ -19,6 +19,7 @@
 import org.slf4j.*;
 
 import java.lang.reflect.*;
+import java.math.BigDecimal;
 import java.net.*;
 import java.util.*;
 
@@ -98,6 +99,11 @@ public static boolean isSimpleType(final Class<?> clazz)
 				Class.class.equals(clazz);
 	}
 
+	public static boolean isComparableType(final Class<?> clazz)
+	{
+		return BigDecimal.class.equals(clazz);
+	}
+
 	public static <T> T freshInstanceOf(final Class<T> clazz)
 	{
 		if (clazz == null)

src/main/java/de/danielbechler/util/Comparables.java

@@ -0,0 +1,38 @@
+/*
+ * Copyright 2012 Daniel Bechler
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package de.danielbechler.util;
+
+/** @author Daniel Bechler */
+public class Comparables
+{
+	private Comparables()
+	{
+	}
+
+	public static <T extends Comparable<T>> boolean isEqualByComparison(final T a, final T b)
+	{
+		if (a != null)
+		{
+			return a.compareTo(b) == 0;
+		}
+		else if (b != null)
+		{
+			return b.compareTo(a) == 0;
+		}
+		return true;
+	}
+}

src/test/java/de/danielbechler/diff/BeanDifferShould.java

@@ -24,8 +24,6 @@
 import org.mockito.Mock;
 import org.testng.annotations.*;
 
-import java.math.BigDecimal;
-
 import static de.danielbechler.diff.node.NodeAssertions.assertThat;
 import static java.util.Arrays.*;
 import static org.hamcrest.MatcherAssert.assertThat;
@@ -99,6 +97,18 @@ public void ignore_ignored_properties()
 		assertThat(node).self().hasState(Node.State.IGNORED);
 	}
 
+    @Test
+    public void compare_bean_via_compare_to()
+    {
+        final ObjectWithCompareTo working = new ObjectWithCompareTo("foo", "ignore");
+        final ObjectWithCompareTo base = new ObjectWithCompareTo("foo", "ignore this too");
+        configuration.withCompareToOnlyType(ObjectWithCompareTo.class);
+
+        final Node node = differ.compare(Node.ROOT, Instances.of(working, base));
+
+        assertThat(node).self().isUntouched();
+    }
+
 	@Test
 	public void compare_bean_via_equals()
 	{
@@ -111,32 +121,6 @@ public void compare_bean_via_equals()
 		assertThat(node).self().isUntouched();
 	}
 
-	@Test
-	public void compare_bean_via_comparable_when_same_value()
-	{
-		final BigDecimal working = new BigDecimal("1.0");
-		final BigDecimal base = new BigDecimal("1.00");
-		configuration.withComparableType(BigDecimal.class);
-
-		final Node node = differ.compare(Node.ROOT, Instances.of(working, base));
-
-		assertThat(node).self().isUntouched();
-	}
-
-	@Test
-	public void compare_bean_via_comparable_when_different_value()
-	{
-		final BigDecimal working = new BigDecimal("1.0");
-		final BigDecimal base = new BigDecimal("1.01");
-		configuration.withComparableType(BigDecimal.class);
-
-		final Node node = differ.compare(Node.ROOT, Instances.of(working, base));
-
-		assertThat(node).self().hasChanges();
-	}
-
-
-
 	@Test
 	public void compare_bean_via_introspection_and_delegate_comparison_of_properties()
 	{