Graph: TreeSet is not adding all the edges

Question

I am working on an undirected graph problem. I have a Node class which contains nodeName and List<Edge>. Then I have Edge class which holds Node start, Node end and weight. And finally I have a Graph class which has one instance variable Map<String, Node>.

I want to add all distinct edges to a set and order them in an increasing order based on their edge weight. Hence I chose TreeSet to store all the edges in an unique sorted fashion. But when I call the addAllEdges() it doesn't add all the edges. The graph has 9 vertices and 14 edges. However the treeSet saves only 10 of those.

Code Structure:

Graph:

class Graph {
    private Map<String, Node> verticesMap = new HashMap<>();

    public void addEdge(String src, String dest, int weight) {
        Node srcNode = verticesMap.get(src) == null ? new Node(src) : verticesMap.get(src);
        Node destNode = verticesMap.get(dest) == null ? new Node(dest) : verticesMap.get(dest);
        Edge edge = new Edge(srcNode, destNode, weight);
        srcNode.getEdgeList().add(edge);
        verticesMap.put(src, srcNode);
        verticesMap.put(dest, destNode);
    }

    public Set<Edge> addAllEdges() {
        Set<Edge> allEdgesSet = new TreeSet<>(getComparatorBasedOnEdgeWeight());
        for (Node node : verticesMap.values()) {
            for (Edge edge : node.getEdgeList()) {
            // sysout for debugging purpose only
                if(allEdgesSet.add(edge)) {
                    System.out.println("Added edge: " + edge);
                } else {
                    System.out.println("Did not add edge: " + edge);
                }
            }
        }
        return allEdgesSet;
    }

    private Comparator<Edge> getComparatorBasedOnEdgeWeight() {
        return (e1, e2) -> Integer.compare(e1.getWeight(), e2.getWeight());
    }
}

Node:

class Node {
    private String name;
    private List<Edge> edgeList;

    public Node(String name) {
        this.name = name;
        edgeList = new ArrayList<>();
    }

    @Override
    public int hashCode() {
        final int prime = 31;
        int result = 1;
        result = prime * result + ((name == null) ? 0 : name.hashCode());
        return result;
    }

    @Override
    public boolean equals(Object obj) {
        if (this == obj)
            return true;
        if (obj == null)
            return false;
        if (getClass() != obj.getClass())
            return false;
        Node other = (Node) obj;
        if (name == null) {
            if (other.name != null)
                return false;
        } else if (!name.equals(other.name))
            return false;
        return true;
    }

    @Override
    public String toString() {
        return name;
    }
}

Edge:

class Edge {
    private Node src;
    private Node dest;
    private int weight;

    public Edge(Node src, Node dest, int weight) {
        this.src = src;
        this.dest = dest;
        this.weight = weight;
    }

    @Override
    public int hashCode() {
        final int prime = 31;
        int result = 1;
        result = prime * result + ((dest == null) ? 0 : dest.hashCode());
        result = prime * result + ((src == null) ? 0 : src.hashCode());
        return result;
    }

    @Override
    public boolean equals(Object obj) {
        if (this == obj)
            return true;
        if (obj == null)
            return false;
        if (getClass() != obj.getClass())
            return false;
        Edge other = (Edge) obj;
        if (dest == null) {
            if (other.dest != null)
                return false;
        } else if (!dest.equals(other.dest))
            return false;
        if (src == null) {
            if (other.src != null)
                return false;
        } else if (!src.equals(other.src))
            return false;
        return true;
    }

    @Override
    public String toString() {
        return src + "-->" + dest + "[" + weight + "]";
    }
}

Test:

 public class Test {
        public static void main(String[] args) {
            Graph graph = new Graph();
            graph.addEdge("A", "B", 4);
            graph.addEdge("B", "A", 4);
            graph.addEdge("A", "I", 8);
            graph.addEdge("I", "A", 8);
            graph.addEdge("B", "C", 8);
            graph.addEdge("C", "B", 8);
            graph.addEdge("B", "I", 11);
            graph.addEdge("I", "B", 11);
            graph.addEdge("C", "H", 2);
            graph.addEdge("H", "C", 2);
            graph.addEdge("C", "D", 7);
            graph.addEdge("D", "C", 7);
            graph.addEdge("C", "F", 4);
            graph.addEdge("F", "C", 4);
            graph.addEdge("H", "I", 7);
            graph.addEdge("I", "H", 7);
            graph.addEdge("H", "G", 6);
            graph.addEdge("G", "H", 6);
            graph.addEdge("I", "G", 1);
            graph.addEdge("G", "I", 1);
            graph.addEdge("G", "F", 2);
            graph.addEdge("F", "G", 2);
            graph.addEdge("F", "D", 14);
            graph.addEdge("D", "F", 14);
            graph.addEdge("F", "E", 10);
            graph.addEdge("E", "F", 10);
            graph.addEdge("E", "D", 9);
            graph.addEdge("D", "E", 9);
            graph.addAllEdges();
        }
    }

Output:

Added edge: A-->B[4]
Added edge: A-->I[9]
Did not add edge: B-->A[4]
Added edge: B-->C[8]
Added edge: B-->I[11]
Did not add edge: C-->B[8]
Added edge: C-->H[2]
Added edge: C-->D[7]
Did not add edge: C-->F[4]
Did not add edge: D-->C[7]
Added edge: D-->F[14]
Did not add edge: D-->E[9]
Added edge: E-->F[10]
Did not add edge: E-->D[9]
Did not add edge: F-->C[4]
Did not add edge: F-->G[2]
Did not add edge: F-->D[14]
Did not add edge: F-->E[10]
Added edge: G-->H[6]
Added edge: G-->I[1]
Did not add edge: G-->F[2]
Did not add edge: H-->C[2]
Did not add edge: H-->I[7]
Did not add edge: H-->G[6]
Did not add edge: I-->A[9]
Did not add edge: I-->B[11]
Did not add edge: I-->H[7]
Did not add edge: I-->G[1]

Edges in the TreeSet:
[G-->I[1], C-->H[2], A-->B[4], G-->H[6], C-->D[7], B-->C[8], A-->I[9], E-->F[10], B-->I[11], D-->F[14]]

The output above shows that it is iterating through 28 edges (14 vertices * 2). I am not able to figure out why treeSet is returning false for those edges which aren't even present in the set yet.

Some of the edges have the same weight. They will only appear once in the resulting set. (Details can be read in other answers). Instead of a `TreeSet`, you should use a `List` (e.g. `ArrayList`), and then sort it with `Collections.sort(list, yourWeightComparator);`. — Marco13, Jul 09 '17 at 17:18
@Marco13: Thanks a lot for your comment. I was reading Andreas answer and you are right about. I will try to solve this problem now. — RDM, Jul 09 '17 at 17:30

score 1 · Accepted Answer · answered Jul 09 '17 at 17:20

TreeSet uses the Comparator for the ordering, but also for the uniqueness of objects. It does not use equals() or hashCode().

Your Comparator needs to compare nodes too, e.g. using node name.

Also, if your graph was truly undirected, you should not need to do graph.addEdge("A", "B", 4) and graph.addEdge("B", "A", 4), since they represent the same undirected edge. An undirected edge has no "source" or "destination".

I would suggest that you change Edge, renaming src and dest to node1 and node2, and ensure that name of node1 is less than name of node2.

That way, your Comparator can then simply compare weight, node1.name, and node2.name:

private Comparator<Edge> getComparatorBasedOnEdgeWeight() {
    return (e1, e2) -> {
        int cmp = Integer.compare(e1.getWeight(), e2.getWeight());
        if (cmp == 0)
            cmp = e1.getNode1().getName().compareTo(e2.getNode1().getName());
        if (cmp == 0)
            cmp = e1.getNode2().getName().compareTo(e2.getNode2().getName());
        return cmp;
    };
}

Thanks a lot for the quick solution. Let me work on it now. Thanks — RDM, Jul 09 '17 at 17:25

Graph: TreeSet is not adding all the edges

1 Answers1