I have three arrays that look like this sample:

The left and right arrays are the index numbers for the left and right children of the current index. If either holds -1, that child doesn't exist. If both hold -1, the node at the index is a leaf. I know this doesn't really make sense to implement in a real-world scenario but this is for an assignment.

Anyways I am trying to implement a remove method within a class that implements this idea. I have it working for everything but the case where a node has two children. My issue here is that I am making a call to a recursive method (all methods I create must be recursive) that is supposed to return the index that holds the largest node in the left subtree of the node I am removing. I need this as this is the node that will be replacing the node with two children that is being removed.

My current code returns the index of the first node it sees in the subtree, as my returns are essentially being overwritten. See here:

//find largest node in left subtree and return its index
private int findNew(int index) {
    int r = right[index];
    if(r != -1) {
        findNew(r);
    }
    return index;
}

Here is the remove method I have implemented:

private void remove(int d, int index) {
    //we found the data to remove
    if(data[index] == d){
    //removes
        //if the node is a leaf
        if(left[index] == -1 && right[index] == -1) {
            data[index] = -1;
            if(free == -1) {
                free = index;
            } else {
                freeUpdate(index);
            }
            currentSize--;
        }
        //the node has a left child
        else if(left[index] != -1 && right[index] == -1) {
            int l = left[index];
            data[index] = data[l];
            left[index] = left[l];
            right[index] = right[l];
            if(free == -1) {
                free = l;
            } else {
                freeUpdate(l);
            }
            //delete stuff in node that moved
            data[l] = -1;
            right[l] = -1;
            currentSize--;
        }
        //the node has a right child
        else if(left[index] == -1 && right[index] != -1) {
            int r = right[index];
            data[index] = data[r];
            left[index] = left[r];
            right[index] = right[r];
            if(free == -1) {
                free = r;
            } else {
                freeUpdate(r);
            }
            //delete stuff in node that moved
            data[r] = -1;
            right[r] = -1;
            currentSize--;
        }
        //the node has two children
        else {
            int l = left[index];
            int r = right[index];
            int newParent = findNew(l);
            //implement the rest of the remove here
            currentSize--;
        }
    } else {
        //if we have searched the entire tree
        if(index == data.length) {
            return;
        } else {
            //keep searching for d
            remove(d, index   1);
        }
    }
}

Attributes and constructor for the class:

private int root;
private int free;
private int left[];
private int data[];
private int right[];
private int currentSize;

public BoundedBST(int size) {
    root = -1;
    free = -1;
    left = new int[size];
    data = new int[size];
    right = new int[size];
    currentSize = 0;
}

public boolean full() {
    return free == -1 && currentSize == data.length;
}

Again I know this implementation does not make much sense, but it's what I am working with. I know the findNew method can be done very easily by using a loop, but I'm not able to do so here.

Essentially I am just trying to find a way for my findNew method to work recursively without overwriting what was returned from the very last call. I understand the error that is occurring here, but I don't know how else to implement such a function that can still have a non-void return type.

CodePudding user response：

The problem with findNew is that when you make the recursive call, you don't do anything with what that recursive call returns. That value is ignored, and you return index in all cases. This makes the recursive call useless.

You should actually capture that return value, and return it again to the caller, so that this index bubbles up out of the recursive calls until it reaches the original caller.

I would also suggest to give this function a more descriptive name: findGreatest:

private int findGreatest(int index) { // Use a better name
    int r = right[index];
    if (r != -1) {
        return findGreatest(r); // Must do something with the value returned!
    }
    return index;
}

This fixes the issue you ask about. But you still need to complete the part where you wrote "implement the rest of the remove here". I suggest you work on that. If you bump into specific problems doing that, feel free to ask a new question about that.

Finally, make sure you test your solution thoroughly, on many different scenario's, before deciding all is good.