cycle detection in undirected graph using disjointsets?

Question

I am implementing the code for cycle detection in undirected graph using find/union methods of disjointsets.

Here is the implementation:

public boolean isCyclicundirected(){
    int k;
    ArrayDisjointSet set = new ArrayDisjointSet(5);
    //Set<Vertex> parents = new HashSet<Vertex>();
    System.out.println(vertexMap);
    Set<String> allVertices = vertexMap.keySet();
    for (String v : allVertices){
        Iterator<Edge> e = vertexMap.get(v).adj.iterator();
        while (e.hasNext()){
            int i = Integer.parseInt(vertexMap.get(v).name);
            int j = Integer.parseInt(e.next().target.name);
            if (set.isConnected(i, j))
                return true;
            else
                   k = set.join(i, j);
            System.out.println(set);
    }
    }
    return false;
}

and here is the isConnected of disjoinset

public boolean isConnected(int i, int j){
    return find(i)==find(j);
}

if two nodes have the same root, (returned by find), that indicates there is a cycle. For a graph like this which has no cycles (1,2),(2,3),(3,4), my method returns true. I am failing to understand what is wrong.

EDIT latest: After suggestions below:

public boolean isCyclicundirected() {
    int k;
    HashSet<HashSet<Vertex>> vertexpairs = new HashSet<HashSet<Vertex>>();
    ArrayDisjointSet set = new ArrayDisjointSet(100);
    Set<String> allVertices = vertexMap.keySet();
    for (String v : allVertices) {
        Vertex current = vertexMap.get(v);
        for (Edge e : current.adj){
            Vertex nextVertex = e.target;
            HashSet<Vertex> temp = new HashSet<Vertex>();
            temp.add(nextVertex);
            temp.add(current);

            if (!vertexpairs.contains(temp)) {
                vertexpairs.add(temp);
                int i = Integer.parseInt(current.name);
                int j = Integer.parseInt(nextVertex.name);
                if (set.isConnected(i, j))
                    return true;
                else
                    k = set.join(i, j);
                System.out.println(set);
            }
        }
    }
    return false;
}

I get node:java.util.NoSuchElementException

score 2 · Accepted Answer · answered Dec 02 '13 at 23:40

2

You iterate over each edge twice, once from each side. You need to only consider any edge once.

answered Dec 02 '13 at 23:40

user2357112

260,549
28
431
505

will that make any difference to the cycle detection? – brain storm Dec 02 '13 at 23:42
@user1988876: Yes. If you iterate over each edge twice, you're basically looking at the graph as if all edges were instead two edges. In the doubled graph, any edge and its double form a cycle. – user2357112 Dec 02 '13 at 23:44
I am considering for each vertex, its neighbours..do I have to have to mark nodes that are visited? to my understanding, disjointset approach need not have to keep track of visited nodes. but I could be totally wrong.. – brain storm Dec 02 '13 at 23:45
@user1988876: No. Marking visited nodes will not help. This algorithm is fundamentally about the edges, not the vertices. If your graph representation does not make it easy to iterate over the edges, you can try keeping track of which edges you've visited and ignoring any edge on the second visit. – user2357112 Dec 02 '13 at 23:49
I now included a field to keep track of edges visited, the code is updated in the edit above, but now I get `node:java.util.NoSuchElementException` – brain storm Dec 02 '13 at 23:57
@user1988876: You're calling `e.next()` twice. Just for-each over the edges instead of using an explicit iterator. Also, it looks like you're using separate edge objects to represent the edge from each side, so marking edges visited with a field in the edge object probably won't work. – user2357112 Dec 03 '13 at 00:00
let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/42368/discussion-between-user1988876-and-user2357112) – brain storm Dec 03 '13 at 00:01

cycle detection in undirected graph using disjointsets?

1 Answers1