Fixed the Distance Comparison bugs by using a new KeyComparator class of type Comparable<NodeId> and using BigIntegers to represent the keys and letting BigInteger do the comparisons

src/kademlia/node/KeyComparator.java

@@ -0,0 +1,44 @@
+package kademlia.node;
+
+import java.math.BigInteger;
+import java.util.Comparator;
+
+/**
+ * A Comparator to compare 2 keys to a given key
+ *
+ * @author Joshua Kissoon
+ * @since 20140322
+ */
+public class KeyComparator implements Comparator<Node>
+{
+
+    private final BigInteger key;
+
+    /**
+     * @param key The NodeId relative to which the distance should be measured.
+     */
+    public KeyComparator(NodeId key)
+    {
+        this.key = key.getInt();
+    }
+
+    /**
+     * Compare two objects which must both be of type <code>Node</code>
+     * and determine which is closest to the identifier specified in the
+     * constructor.
+     *
+     * @param n1 Node 1 to compare distance from the key
+     * @param n2 Node 2 to compare distance from the key
+     */
+    @Override
+    public int compare(Node n1, Node n2)
+    {
+        BigInteger b1 = n1.getNodeId().getInt();
+        BigInteger b2 = n2.getNodeId().getInt();
+
+        b1 = b1.xor(key);
+        b2 = b2.xor(key);
+
+        return b1.abs().compareTo(b2.abs());
+    }
+}

src/kademlia/node/Node.java

@@ -158,79 +158,4 @@ public class Node implements Streamable
     {
         return this.getNodeId().toString();
     }
-
-    /**
-     * A DistanceComparator is used to compare Node objects based on their closeness
-     * */
-    public static class DistanceComparator implements Comparator
-    {
-
-        private final NodeId nid;
-
-        /**
-         * The NodeId relative to which the distance should be measured.
-         *
-         * @param nid
-         * */
-        public DistanceComparator(NodeId nid)
-        {
-            this.nid = nid;
-        }
-
-        /**
-         * Compare two objects which must both be of type <code>Node</code>
-         * and determine which is closest to the identifier specified in the
-         * constructor.
-         * */
-        @Override
-        public int compare(Object o1, Object o2)
-        {
-            Node n1 = (Node) o1;
-            Node n2 = (Node) o2;
-//            System.out.println("\nDistance Comparator: " + nodeId);
-//            System.out.println("Distance Comparator: " + n1.getNodeId());
-//            System.out.println("Distance Comparator: " + n2.getNodeId());
-
-            /* Check if they are equal and return 0 */
-//            if (n1.equals(n2))
-//            {
-//                //System.out.println("Distance Comparator: Return 0");
-//                return 0;
-//            }
-            //System.out.println("\n **************** Compare Starting **************** ");
-            //System.out.println("Comparing to: " + this.nodeId);
-            int distance1 = nid.getDistance(n1.getNodeId());
-            //System.out.println("Node " + n1.getNodeId() + " distance: " + index1);
-            int distance2 = nid.getDistance(n2.getNodeId());
-            //System.out.println("Node " + n2.getNodeId() + " distance: " + index2);
-
-            int retval;
-
-            if ((distance1 == distance2) && n1.equals(n2))
-            {
-                /**
-                 * If the first node is farther to the given node, return 1
-                 *
-                 * @note -1 will also be returned if both nodes are the same distance away
-                 * This really don't make a difference though, since they need to be sorted.
-                 */
-                //System.out.println("Distance Comparator: Return -1");
-                retval = 0;
-            }
-            else if (distance1 < distance2)
-            {
-                /* If the first node is closer to the given node, return 1 */
-                //System.out.println("Distance Comparator: Return 1");
-                retval = 1;
-            }
-            else
-            {
-                return -1;
-            }
-
-            //System.out.println("Returned: " + retval);
-            //System.out.println("**************** Compare Ended ***************** \n");
-            return retval;
-        }
-    }
 }

src/kademlia/node/NodeId.java

@@ -74,6 +74,14 @@ public class NodeId implements Streamable
         return this.keyBytes;
     }
 
+    /**
+     * @return The BigInteger representation of the key
+     */
+    public BigInteger getInt()
+    {
+        return new BigInteger(1, this.getBytes());
+    }
+
     /**
      * Compares a NodeId to this NodeId
      *
@@ -115,12 +123,10 @@ public class NodeId implements Streamable
         for (int i = 0; i < ID_LENGTH / 8; i++)
         {
             result[i] = (byte) (this.keyBytes[i] ^ nidBytes[i]);
-            //System.out.println("XOR Result: " + result[i]);
         }
 
         NodeId resNid = new NodeId(result);
 
-        //System.out.println("Resulting Nid: " + resNid + " First set bit: " + resNid.getFirstSetBitIndex());
         return resNid;
     }
 

src/kademlia/operation/ContentLookupOperation.java

@@ -19,6 +19,7 @@ import kademlia.message.ContentLookupMessage;
 import kademlia.message.ContentMessage;
 import kademlia.message.Message;
 import kademlia.message.NodeReplyMessage;
+import kademlia.node.KeyComparator;
 import kademlia.node.Node;
 
 /**
@@ -82,7 +83,7 @@ public class ContentLookupOperation implements Operation, Receiver
          * We initialize a TreeMap to store nodes.
          * This map will be sorted by which nodes are closest to the lookupId
          */
-        this.comparator = new Node.DistanceComparator(params.getKey());
+        this.comparator = new KeyComparator(params.getKey());
         this.nodes = new TreeMap(this.comparator);
     }
 

src/kademlia/operation/NodeLookupOperation.java

@@ -15,6 +15,7 @@ import kademlia.exceptions.UnknownMessageException;
 import kademlia.message.Message;
 import kademlia.message.NodeLookupMessage;
 import kademlia.message.NodeReplyMessage;
+import kademlia.node.KeyComparator;
 import kademlia.node.Node;
 import kademlia.node.NodeId;
 
@@ -72,9 +73,9 @@ public class NodeLookupOperation implements Operation, Receiver
          * We initialize a TreeMap to store nodes.
          * This map will be sorted by which nodes are closest to the lookupId
          */
-        this.comparator = new Node.DistanceComparator(lookupId);
+        this.comparator = new KeyComparator(lookupId);
-        //this.nodes = new TreeMap(this.comparator);
+        this.nodes = new TreeMap(this.comparator);
-        this.nodes = new HashMap<>();
+        //this.nodes = new HashMap<>();
     }
 
     /**

src/kademlia/tests/SaveStateTest.java

@@ -1,7 +1,5 @@
 package kademlia.tests;
 
-import java.io.IOException;
-import kademlia.core.Configuration;
 import kademlia.core.Kademlia;
 import kademlia.node.NodeId;
 
@@ -19,51 +17,51 @@ public class SaveStateTest
         try
         {
             /* Setting up 2 Kad networks */
-            Kademlia kad1 = new Kademlia("JoshuaK", new NodeId("ASF45678947584567467"), 12049);
+            Kademlia kad1 = new Kademlia("JoshuaK", new NodeId("ASF45678947584567463"), 12049);
-            Kademlia kad2 = new Kademlia("Crystal", new NodeId("AfERTKdvHGVHERJHGFdh"), 4585);
+            Kademlia kad2 = new Kademlia("Crystal", new NodeId("ASF45678947584567464"), 4585);
-            Kademlia kad3 = new Kademlia("Shameer", new NodeId("ASERTKyrHGVHERfHGFsy"), 8104);
+            Kademlia kad3 = new Kademlia("Shameer", new NodeId("ASF45678947584567465"), 8104);
-            Kademlia kad4 = new Kademlia("Lokesh", new NodeId("AS3RTKJsdjVHERJHGF94"), 8335);
+            Kademlia kad4 = new Kademlia("Lokesh", new NodeId("ASF45678947584567466"), 8335);
-            Kademlia kad5 = new Kademlia("Chandu", new NodeId("ASERT47kfeVHERJHGF15"), 13345);
+            Kademlia kad5 = new Kademlia("Chandu", new NodeId("ASF45678947584567467"), 13345);
-            
+
             /* Connecting 2 to 1 */
             System.out.println("Connecting Nodes 1 & 2");
-            kad2.bootstrap(kad1.getNode());            
+            kad2.bootstrap(kad1.getNode());
             System.out.println(kad1);
             System.out.println(kad2);
-            
+
-            kad3.bootstrap(kad2.getNode());            
+            kad3.bootstrap(kad2.getNode());
             System.out.println(kad1);
             System.out.println(kad2);
-            System.out.println(kad3);            
+            System.out.println(kad3);
-            
+
-            kad4.bootstrap(kad2.getNode());            
+            kad4.bootstrap(kad2.getNode());
             System.out.println(kad1);
             System.out.println(kad2);
             System.out.println(kad3);
             System.out.println(kad4);
-            
+
             kad5.bootstrap(kad4.getNode());
-            
+
             System.out.println(kad1);
             System.out.println(kad2);
             System.out.println(kad3);
             System.out.println(kad4);
             System.out.println(kad5);
-            
+
             synchronized (this)
             {
                 System.out.println("\n\n\n\nSTORING CONTENT 1\n\n\n\n");
                 DHTContentImpl c = new DHTContentImpl(kad2.getOwnerId(), "Some Data");
                 System.out.println(c);
-                kad2.put(c);                
+                kad2.put(c);
             }
-                        
+
             synchronized (this)
             {
                 System.out.println("\n\n\n\nSTORING CONTENT 2\n\n\n\n");
                 DHTContentImpl c2 = new DHTContentImpl(kad2.getOwnerId(), "Some other Data");
                 System.out.println(c2);
-                kad4.put(c2);              
+                kad4.put(c2);
             }
 
             System.out.println(kad1);
@@ -71,7 +69,7 @@ public class SaveStateTest
             System.out.println(kad3);
             System.out.println(kad4);
             System.out.println(kad5);
-            
+
             /* Shutting down kad1 and restarting it */
             System.out.println("\n\n\nShutting down Kad instance");
             System.out.println(kad2);
@@ -80,11 +78,8 @@ public class SaveStateTest
             System.out.println("\n\n\nReloading Kad instance from file");
             Kademlia kadR2 = Kademlia.loadFromFile("JoshuaK");
             System.out.println(kadR2);
-        }        
+        }
-//        catch(RoutingException e)
+        
-//        {
-//            System.err.println("Routing Exception");
-//        }
         catch (Exception e)
         {
             e.printStackTrace();

src/kademlia/tests/TreeMapTest.java

@@ -6,6 +6,7 @@ import java.util.Map;
 import java.util.SortedMap;
 import java.util.TreeMap;
 import kademlia.core.Kademlia;
+import kademlia.node.KeyComparator;
 import kademlia.node.Node;
 import kademlia.node.NodeId;
 
@@ -38,7 +39,7 @@ public class TreeMapTest
         Kademlia kad4 = new Kademlia("Lokesh", new NodeId("AS3RTKJsdjVHERJHGF94"), 8891);
         Kademlia kad5 = new Kademlia("Chandu", new NodeId("ASERT47kfeVHERJHGF15"), 8892);
 
-        this.comparator = new Node.DistanceComparator(kad1.getNode().getNodeId());
+        this.comparator = new KeyComparator(kad1.getNode().getNodeId());
         this.nodes = new TreeMap(this.comparator);
 
         /* Add all nodes as unasked */