Inconsistent equals/hashCode methods
jolkdarr opened this issue · 9 comments
For instance, the class Graph.CostPathPair is not correct:
import java.util.HashSet;
import org.junit.Test;
import com.jwetherell.algorithms.data_structures.Graph;
import com.jwetherell.algorithms.data_structures.Graph.*;
public class GraphTest {
@Test
public void testCostVertexPair() {
HashSet<Edge<Integer>> s1 = new HashSet<Graph.Edge<Integer>>();
s1.add(new Edge<Integer>(1, new Vertex<Integer>(10), new Vertex<Integer>(20)));
s1.add(new Edge<Integer>(2, new Vertex<Integer>(20), new Vertex<Integer>(30)));
Graph.CostPathPair<Integer> p1 = new Graph.CostPathPair<Integer>(1, s1);
HashSet<Edge<Integer>> s2 = new HashSet<Graph.Edge<Integer>>();
s2.add(new Edge<Integer>(2, new Vertex<Integer>(10), new Vertex<Integer>(20)));
s2.add(new Edge<Integer>(1, new Vertex<Integer>(20), new Vertex<Integer>(30)));
Graph.CostPathPair<Integer> p2 = new Graph.CostPathPair<Integer>(1, s2);
System.err.println(p1.equals(p2)); // => false
System.err.println(p2.equals(p1)); // => false
System.err.println(p1.hashCode()); // 62
System.err.println(p2.hashCode()); // 62
}
}(question arising while testing: Shouldn't the class Graph be moved in the graph package?)
I'll look into the hash/equals mismatch. It's likely because I assume the path to be linear where a Set has no such concept. I'll clean it up in the next couple of days.
Also, on the topic of Graph: are you suggesting that I move into into a 'graph' sub-package of data_structures or move it out of data structures into a package at the same level as data_structures?
Ok, I wait for the fix... :)
I think I have understood now:
The class Graph is defined in data_structures with all other data types.
The graph package only contains algorithms related to graphs.
OK, I have fixed the path related equals/hash method. Let me know if you believe it is fixed also. Thanks for helping make this a better repository.
Yes, the graph classes layout isn't the most straightforward.
Problem still present. Given two graphs g1 and g2,
!g1.equals(g2) should imply g1.hashCode() != g2.hashCode()
g1.hashCode() == g2.hashCode() should imply g1.equals(g2)
I don't think the first statement is correct, from the Java docs.
"It is not required that if two objects are unequal according to the equals(java.lang.Object) method, then calling the hashCode method on each of the two objects must produce distinct integer results."
http://docs.oracle.com/javase/7/docs/api/java/lang/Object.html
If you don't provide hashCode/equals "consistently", you should document it in javadocs because your classes can't be used in hash maps or hash sets etc etc...
Because I actually need consistency, I have switched to the JGraphT library instead.
Location: https://github.com/jgrapht/jgrapht/tree/master/jgrapht-core/src/main/java/org/jgrapht
Thanks for your time.
My point is, the class is consistent with the equals/hashCode javadoc
contract. All the hashing data structures use hashCode as their first
filter, if two objects have the same hash code then it uses equals to
determine the correct object.
Since the objects equals method ultimately determine the equality the path
objects will work as expected in all the java collection/maps.
On Jan 6, 2016 5:07 PM, "jolkdarr" notifications@github.com wrote:
If you don't provide hashCode/equals "consistently", you should document
it in javadocs because your classes can't be used in hash maps or hash sets
etc etc...Because I actually need consistency, I have switched to the JGraphT
library instead.
Location:
https://github.com/jgrapht/jgrapht/tree/master/jgrapht-core/src/main/java/org/jgraphtThanks for your time.
—
Reply to this email directly or view it on GitHub
#16 (comment)
.
You are right. Your classes shall not avoid collisions.
Because I use hashcodes as keys for performance reasons, I can't use that library.
With JGraphT all works as I expect.
Thank you.
OK @jolkdarr thanks for the feedback. I can change the hashCode() fairly easily if that will help.