Brodie
Brodie

Reputation: 63

Find child in a java Tree(recursively)

thanks for check it out. What I want is to get the tree where root id is the id passed by param but its returning null pointer.

Tree settings

TreeIF<DoctorIF> tree = new Tree<DoctorIF>();

   setTree(tree);

   DoctorS ceo = new DoctorS(1, this);
   DoctorS two = new DoctorS(2, this);
   DoctorS three = new DoctorS(3, this);
   DoctorS four = new DoctorS(4, this);
   DoctorS five = new DoctorS(5, this);
   DoctorS six = new DoctorS(6, this);
   DoctorS seven = new DoctorS(7, this);

   TreeIF<DoctorIF> t1 = new Tree<DoctorIF>(three);
   TreeIF<DoctorIF> t2 = new Tree<DoctorIF>(four);
   TreeIF<DoctorIF> t3 = new Tree<DoctorIF>(two);
   tree.setRoot(ceo);

   t1.addChild(new Tree<DoctorIF>(seven));
   t2.addChild(new Tree<DoctorIF>(five));
   t3.addChild(t1);
   t3.addChild(t2);
   t3.addChild(new Tree<DoctorIF>(six));

   tree.addChild(t3);

Function to get tree or tree child. I got nullpointer when I'm searching id {4,5,6,7}.

private TreeIF<DoctorIF> getTreeChild(TreeIF<DoctorIF> tree_, int id){
    if (tree_.getRoot().getId()==id && id!=1)return tree_;
    else{
        ListIF<TreeIF<DoctorIF>> list = tree_.getChildren();
        IteratorIF<TreeIF<DoctorIF>> it = list.getIterator();
        while(it.hasNext()){
            TreeIF<DoctorIF> subtree = getTreeChild(it.getNext(), id);
            if (subtree.getRoot()!=null && subtree.getRoot().getId()==id)return subtree;
        }
    }
    return null;
}

 @Override
public DoctorIF getDoctor(int id){
    TreeIF<DoctorIF> tree_ = null;
    tree_ = getTreeChild(tree, id);
    return tree_.getRoot();
}

Upvotes: 0

Views: 148

Answers (2)

Jorn Vernee
Jorn Vernee

Reputation: 33905

Your function is needlessly complex. It returns either the subtree if it was found, or null otherwise. So you don't have to check the id twice.

getTreeChild can return null, but you dereference it without checking, so that's probably where the null pointer exception comes from.

This should work:

private TreeIF<DoctorIF> getTreeChild(TreeIF<DoctorIF> tree_, int id){
    if (tree_.getRoot().getId()==id && id!=1)return tree_;
    else{
        ListIF<TreeIF<DoctorIF>> list = tree_.getChildren();
        IteratorIF<TreeIF<DoctorIF>> it = list.getIterator();
        while(it.hasNext()){
            TreeIF<DoctorIF> subtree = getTreeChild(it.getNext(), id);
            if (subtree != null) return subtree; // No need to check the id again.
        }
    }
    return null;
}

Your code also looks a little messy, with the Iterator like this. You can use a java for-each loop if you have your ListIF implement Iterable<...> and have your IteratorIF implement Iterator<...>.

// ListIF<TreeIF<DoctorIF>> list = tree_.getChildren();
// IteratorIF<TreeIF<DoctorIF>> it = list.getIterator();
// while(it.hasNext()){
//     TreeIF<DoctorIF> subtree = getTreeChild(it.getNext(), id);
//     if (subtree != null) return subtree;
// }

for(TreeIF<DoctorIF> child : tree_.getChildren()) {
    TreeIF<DoctorIF> subtree = getTreeChild(child, id);
    if (subtree != null) return subtree; 
}

Upvotes: 1

Daniel Szalay
Daniel Szalay

Reputation: 4101

I have added some comments to explain my changes:

private TreeIF<DoctorIF> getTreeChild(TreeIF<DoctorIF> tree_, int id){
    if (tree_.getRoot().getId()==id && id!=1)return tree_;
    else{
        ListIF<TreeIF<DoctorIF>> list = tree_.getChildren();
        IteratorIF<TreeIF<DoctorIF>> it = list.getIterator();
        while(it.hasNext()){
            TreeIF<DoctorIF> subtree = getTreeChild(it.getNext(), id);
            //subtree might be null at this point
            //In that case, subtree.getRoot() will throw a NullPointerException
            if (subtree == null) continue; //This will prevent the NullPointerException
            if (subtree.getRoot()!=null && subtree.getRoot().getId()==id)return subtree;
        }
    }
    return null;
}

Upvotes: 1

Related Questions