corrected handling of null value in the TreeNodeComparator. Previous …

…if any node or atom were null the comparator would return '0' which is incorrect. If one is null and the other is not the objects are not equal. Change-Id: I928a6832ec15cdf5fad67a9dec41bcec51b05db1 Signed-off-by: Egon Willighagen <egonw@users.sourceforge.net>
cdk · Dec 12, 2012 · d7a55d3 · d7a55d3
1 parent 5a5db14
commit d7a55d3
Showing 1 changed file with 25 additions and 16 deletions.
diff --git a/src/main/org/openscience/cdk/tools/HOSECodeGenerator.java b/src/main/org/openscience/cdk/tools/HOSECodeGenerator.java
@@ -47,6 +47,7 @@
 import org.openscience.cdk.interfaces.IRing;
 import org.openscience.cdk.interfaces.IRingSet;
 import org.openscience.cdk.ringsearch.SSSRFinder;
+import org.openscience.cdk.smiles.InvPair;
 
 /**
  * Generates HOSE codes {@cdk.cite BRE78}.
@@ -733,25 +734,33 @@ class TreeNodeComparator implements Comparator<TreeNode> {
     /**
      *The compare method, compares by canonical label of atoms
      *
-     * @param  obj1  The first TreeNode
-     * @param  obj2  The second TreeNode
+     * @param  a  The first TreeNode
+     * @param  b  The second TreeNode
      * @return       -1,0,1
      */
-    public int compare(TreeNode obj1, TreeNode obj2) {
-    	if(obj1==null || obj2==null || ((TreeNode) obj1).getAtom()==null || ((TreeNode) obj2).getAtom()==null)
-    		return 0;
-    	Long label1 = (Long)((TreeNode) obj1).getAtom().getProperty("CanonicalLable");
-    	Long label2 = (Long)((TreeNode) obj2).getAtom().getProperty("CanonicalLable");
-    	if(label1==null || label2==null)
-    		return 0;
-    	if (label1.intValue() < label2.intValue()) {
-    		return (-1);
-    	}
-    	if (label1.intValue() > label2.intValue()) {
-    		return (1);
-    	}
-    	return (0);
+    public int compare(TreeNode a, TreeNode b) {
+        return label(a).compareTo(label(b));
     }
+
+    /**
+     * Access the canonical label for the given tree node's atom. If any component is null
+     * then {@link Long#MIN_VALUE} is return thus sorting that object in lower order.
+     * @param node a tree node to get the label from
+     * @return canonical label value
+     */
+    private Long label(TreeNode node){
+        if(node == null)
+            return Long.MIN_VALUE;
+        IAtom atom = node.getAtom();
+        if(atom == null)
+            return Long.MIN_VALUE;
+        // cast can be removed in master
+        Long label = (Long) atom.getProperty(InvPair.CANONICAL_LABEL);
+        if(label == null)
+            return Long.MIN_VALUE;
+        return label;
+    }
+
   }
 
   /**