Problem while implementing Find() method for a disjoint set

Question

Been trying to solve this for a while now. I've been given a Node in a Disjoint Set.The Node's implementation is as a class where either it has a different parent node or is it's own

class Node {
    Node parent;
    int rank;
    //...constructor and method declarations}

Now, I have to implement the Find method for this class with path compression, called the getParent() method. I wrote the following code:

Node getParent(){
    while(parent != this)
    {
        parent = parent.getParent();  
    }
    return parent;

I get infinite recursion with this code. I know it's problematic, but I do not know how to fix it. I tried using the this keyword, but it led to the same errors. Please help. Also, an insight on how to write this code in an iterative manner would be a great plus. Also, I would like to restructure the tree such that all intermediate nodes between the node on which the getParent() is called and the root have a common parent i.e. the root node.

I don't understand why you use `while` statement and just not `return parent` in `getParent()` method? — Mohsen, Dec 13 '18 at 07:37
What is the aim of `getParent()` method? returning the first parent of a node or finding the root of parents? — Hamid Ghasemi, Dec 13 '18 at 07:51
@hamidghasemi the aim is to find the root of all parents along a branch and assign that root to all children, grandchildren etc. along the branch — Vedant Pathak, Dec 13 '18 at 08:27
@VedantPathak The first part of your question is answered by Stephen (to find the root of all elements in a branch). In the second part of your question (assign that root to all children, grandchildren etc. along the branch), do you want to destroy the current tree structure and create a tree with a maximum depth of 1? or I misunderstood your requirement? — Hamid Ghasemi, Dec 13 '18 at 08:38
@hamidghasemi yes that is correct I do want to create a tree with a maximum depth of 1 — Vedant Pathak, Dec 13 '18 at 08:50
@VedantPathak I've added the answer, please update your question and add the second part to it :) — Hamid Ghasemi, Dec 13 '18 at 10:16

Stephen C · Answer 1 · 2018-12-13T10:40:27.610

Node getParent() {
    while (parent != this) {
        parent = parent.getParent();  
    }
    return parent;
}

The problems:

You are mutating the parent field.
You should be testing if the parent's parent is itself ... not this.

Your code should probably be:

Node getParent() {
    Node p = parent;
    while(p.parent != p) {
        p = p.parent;  
    }
    return p;
}

Or recursively:

Node getParent() {
    return (parent == this) ? this : parent.getParent();
}

(In this case, the iterative version is safer. You should not get "infinite" recursion, but there is still a risk that you may recurse too deep, with a pathologically deep data structure.)

UPDATE - apparently, it is your intention that getParent should (incrementally) collapse the parent chain of each Node it visit. In that case, the recursive solution would be:

Node getParentAndCollapse() {
    if (parent == this) {
        return this;
    } else {
        parent = parent.getParentAndCollapse();
        return parent;
    }
}

An iterative solution that collapses the entire chain requires two traversals of the chain:

Traverse the chain find the ultimate parent.
Traverse the chain again, updating all the parent fields to refer to the ultimate parent.
Return the ultimate parent.

Something like this

Node getParentAndCollapse() {
    Node p = getParent();  // iterative version
    Node n = this;
    while (n.parent != p) {
        Node np = n.parent;
        n.parent = p;
        n = np;
    }
    return p;
}

I understand the looking for parent's parent part, but why does mutating the parent's value result in problems? shouldn't the compiler branch off to the `parent.getParent()` method call and return only when it has found the root? — Vedant Pathak, Dec 13 '18 at 08:42
Yes I want the parent fields of all nodes along a branch to point to the root after the method call finishes executing. — Vedant Pathak, Dec 13 '18 at 10:16
You need to fix your Question. You have shown us some some buggy code and asked "what is the problem" without explaining clearly (in the Question) what the code is supposed to do, — Stephen C, Dec 13 '18 at 10:21

Hamid Ghasemi · Answer 2 · 2018-12-13T10:29:08.773

0

The first part of your question is already answered by Stephen, and I think this is complete enough.

About the second part of your question, I assume you have all the nodes in a list. So you have List<Node> allNodes. The needed method to restructure the tree would be:

public void reStructureTree(List<Node> allNodes){
    for(Node item: allNodes){
        Node p = item.parent;
        List<Node> branch = new ArrayList<>();

        while (p.parent != p){
            branch.add(p);
            p = p.parent;
        }

        for(Node branchItem : branch){
            branchItem.parent = p;
        }
    }
}

The order of this method in the worst case would be O(n^2) and I did not look for a better way, cause you didn't mention the importance of it.

edited Dec 13 '18 at 10:29

answered Dec 13 '18 at 10:15

Hamid Ghasemi

880
3
13
32

I do not have a list of nodes, but I think that is unimportant as you've used a new list to store all intermediate nodes anyway. also, yeah, time complexity for find is important and I missed out on that, but the problem I'm working on is structured in such a way that it won't get to worst case as getParent() won't be called on all nodes. – Vedant Pathak Dec 13 '18 at 10:27
@VedantPathak It was a bug in the code and I updated it. In the above method, you will pick each node, and update the parent of this node and all intermediate ones between this node and the root. And I think in this way you will have the best possible complexity cause it processes each node only one time. – Hamid Ghasemi Dec 13 '18 at 10:40

Problem while implementing Find() method for a disjoint set

2 Answers2