Delete all entries with a given value in AVL Tree

Question

I am implementing an AVL Tree of the following type AVLTree<K extends Comparable<K>,V> basically every node of the Tree is a Key-Value Node, at the moment i am struggling with a method that deletes all the V values. This is what i have:

private void deleteAll(AVLTreeNode<K,V> root, V value){
        if(root == null) return;
        deleteAll(root.left, value);
        deleteAll(root.right, value);
        if(root.getValue().equals(value)){
            delete(root.getKey());
        }
}

Super easy approach (maybe not optimized O(nlog(n)) if you have a better optimized solution feel free to write it) using a bottom-up recursion.

The delete method:

public void delete(K key){
        super.setRoot(delete(super.getRoot(), key));
}
private AVLTreeNode<K,V> delete(AVLTreeNode<K,V> root, K key) {
        if (root == null) {
            return root;
        } else if (root.getKey().compareTo(key) > 0) {
            root.setLeft(delete(root.left, key));
        } else if (root.getKey().compareTo(key) < 0) {
            root.setRight(delete(root.right, key));
        } else {
            if (root.left == null || root.right == null) {
                root = (root.left == null) ? root.right : root.left;
            } else {
                AVLTreeNode<K,V> mostLeftChild = AVLTreeUtils.minValueNode(root.right);
                root.key = mostLeftChild.getKey();
                root.value = mostLeftChild.getValue();
                root.setRight(delete(root.right, root.key));
            }
            size--;
        }
        if (root != null) {
            root = AVLTreeUtils.rebalance(root);
        }
        return root;
}

What happens is that it deletes correclty most of the values but sometimes it lefts some of the values it had to delete there, here is one example:

DELETING ALL VALUES: 2

BEFORE DELETION OF EVERY: 2

               ->2
          ->0
     ->1
               ->2
          ->1
               ->1
->1
          ->0
     ->2
          ->2

AFTER DELETION

               ->2
          ->0
     ->1
          ->2
->1
          ->1
     ->1
          ->0
Time used: 0 ms tree size: 8
-------------------------------------------

Does tree contain value: 2? true

EDIT:

Here is all the code of the class

package structure.tree.balanced;

import structure.array.Array;
import structure.node.Node;
import structure.tree.Tree;

public class AVLTree<K extends Comparable<K>,V> extends Tree<AVLTree.AVLTreeNode<K, V>> {

    private static final long serialVersionUID = 5046115177325966348L;

    public boolean containsValue(V value) {
        return containsValue(((AVLTreeNode<K,V>) super.getRoot()), value);
    }

    private boolean containsValue(AVLTreeNode<K,V> root, V value){
        if(root == null) return false;
        else {
            if(root.getValue().equals(value)) return true;
            else return containsValue(root.getLeft(), value) || containsValue(root.getRight(), value);
        }
    }

    public boolean containsKey(K key){
        return containsKey(((AVLTreeNode<K,V>) super.getRoot()), key);
    }

    private boolean containsKey(AVLTreeNode<K,V> root, K key){
        if(root == null) return false;
        else {
            if(root.getKey().compareTo(key) < 0) return containsKey(root.getRight(), key);
            else if(root.getKey().compareTo(key) > 0) return containsKey(root.getLeft(), key);
            else return true;
        }
    }

    public void delete(K key){
        super.setRoot(delete(super.getRoot(), key));
    }

    private AVLTreeNode<K,V> delete(AVLTreeNode<K,V> root, K key) {
        if (root == null) {
            return root;
        } else if (root.getKey().compareTo(key) > 0) {
            root.setLeft(delete(root.left, key));
        } else if (root.getKey().compareTo(key) < 0) {
            root.setRight(delete(root.right, key));
        } else {
            if (root.left == null || root.right == null) {
                root = (root.left == null) ? root.right : root.left;
            } else {
                AVLTreeNode<K,V> mostLeftChild = minValueNode(root.right);
                root.key = mostLeftChild.getKey();
                root.value = mostLeftChild.getValue();
                root.setRight(delete(root.right, root.key));
            }
            if(size > 0) size--;
        }
        if (root != null) {
            root = rebalance(root);
        }
        return root;
    }

    public void deleteAll(V value){
        deleteAll(super.getRoot(), value);
    }

    private void deleteAll(AVLTreeNode<K,V> root, V value){
        if(root == null) return;
        deleteAll(root.left, value);
        deleteAll(root.right, value);
        if(root.getValue().equals(value)) delete(root.getKey());
    }

    public V get(K key){
        return get(super.getRoot(), key);
    }

    private V get(AVLTreeNode<K, V> root, K key) {
        if(root.getKey().compareTo(key) == 0) return root.getValue();
        else if(root.getKey().compareTo(key) > 0) return get(root.getLeft(), key);
        else return get(root.getRight(), key);
    }

    private int getBalance(AVLTreeNode<K,V> n) {
        return (n == null) ? 0 : height(n.getRight()) - height(n.getLeft());
    }

    private int height(AVLTreeNode<K,V> n) {
        return n == null ? -1 : n.getHeight();
    }

    public void insert(K key, V value) {
        size++;
        super.setRoot(insert(super.getRoot(), key, value));
    }

    private AVLTreeNode<K,V> insert(AVLTreeNode<K,V> root, K key, V value){
        if (root == null) {
            return new AVLTreeNode<K,V>(key, value);
        } else if (root.getKey().compareTo(key) > 0) {
            root.setLeft(insert(root.getLeft(), key, value));
        } else if (root.getKey().compareTo(key) < 0) {
            root.setRight(insert(root.getRight(), key, value));
        } else {
            size--;
            throw new RuntimeException("duplicate Key!");
        }
        return rebalance(root);
    }

    private AVLTreeNode<K,V> minValueNode(AVLTreeNode<K,V> root){
        if(root == null) return null;
        else {
            if(root.getLeft() != null) return minValueNode(root.getLeft());
            else return root;
        }
    }

    private AVLTreeNode<K,V> rebalance(AVLTreeNode<K,V> z) {
        updateHeight(z);
        int balance = getBalance(z);
        if (balance > 1) {
            if (height(z.getRight().getRight()) > height(z.getRight().getLeft())) {
                z = rotateLeft(z);
            } else {
                z.setRight(rotateRight(z.getRight()));
                z = rotateLeft(z);
            }
        } else if (balance < -1) {
            if (height(z.getLeft().getLeft()) > height(z.getLeft().getRight()))
                z = rotateRight(z);
            else {
                z.setLeft(rotateLeft(z.getLeft()));
                z = rotateRight(z);
            }
        }
        return z;
    }

    public void replace(K key, V newValue) {
        replace(super.getRoot(), key, newValue);
    }

    private void replace(AVLTreeNode<K, V> root, K key, V newValue) {
        if(root != null){
            if(root.getKey().compareTo(key) == 0) root.setValue(newValue);
            else if(root.getKey().compareTo(key) > 0) replace(root.getLeft(), key, newValue);
            else replace(root.getRight(), key, newValue);
        }
    }

    public void replaceAll(V oldValue, V newValue){
        replaceAll(super.getRoot(), oldValue, newValue);
    }

    private void replaceAll(AVLTreeNode<K, V> root, V oldValue, V newValue){
        if(root == null) return;
        if(root.getValue().equals(oldValue)) root.setValue(newValue);
        replaceAll(root.left, oldValue, newValue);
        replaceAll(root.right, oldValue, newValue);
    }

    private AVLTreeNode<K,V> rotateLeft(AVLTreeNode<K,V> y){
        AVLTreeNode<K,V> x = y.getRight();
        AVLTreeNode<K,V> z = x.getLeft();
        x.setLeft(y);
        y.setRight(z);
        updateHeight(y);
        updateHeight(x);
        return x;
    }

    private AVLTreeNode<K,V> rotateRight(AVLTreeNode<K,V> y){
        AVLTreeNode<K,V> x = y.getLeft();
        AVLTreeNode<K,V> z = x.getRight();
        x.setRight(y);
        y.setLeft(z);
        updateHeight(y);
        updateHeight(x);
        return x;
    }

    public Object[] toArray(){
        return toArray(super.getRoot(), new Array<>(size()));
    }

    private Object[] toArray(AVLTreeNode<K,V> root, Array<AVLTreeNode<K,V>> arr){
        if(root != null){
            toArray(root.left, arr);
            arr.insert(root);
            toArray(root.right, arr);
        }
        return arr.toArray();
    }

    private void updateHeight(AVLTreeNode<K,V> root){
        root.setHeight(1 + Math.max(height(root.getLeft()), height(root.getRight())));
    }

    public static class AVLTreeNode<K extends Comparable<K>, V> implements Node<V> {

        private static final long serialVersionUID = -2099221188924293375L;
        private AVLTreeNode<K,V> left, right;
        private K key;
        private V value;
        private int height = 0;
    
        public AVLTreeNode(K key, V value){
            this.key = key;
            this.value = value;
        }
    
        public K getKey() {
            return key;
        }
    
        @SuppressWarnings("unused")
        private void setKey(K key) {
            this.key = key;
        }
    
        public AVLTreeNode<K,V> getLeft() {
            return left;
        }
    
        protected void setLeft(AVLTreeNode<K,V> left) {
            this.left = left;
        }
    
        public AVLTreeNode<K,V> getRight() {
            return right;
        }
    
        protected void setRight(AVLTreeNode<K,V> right) {
            this.right = right;
        }
    
        @Override
        public String toString() {
            return "{\"key\":"+key+",\"value\":"+getValue()+"}";
        }
    
        @Override
        public V getValue() {
            return value;
        }
    
        @Override
        public void setValue(V value) {
            this.value = value;
        }
    
        public int getHeight() {
            return height;
        }
    
        protected void setHeight(int height) {
            this.height = height;
        }
    
        public int getBalance() {
            return (left != null ? left.getHeight() : 0) - (right != null ? right.getHeight() : 0); 
        }
        
    }

}

The superclass is this:

package structure.tree;

import structure.Structure;
import structure.node.Node;

/**
 * A Tree is a {@Structure} that uses {@Node} as value.
 */
public abstract class Tree<N extends Node<?>> {
    
    private static final long serialVersionUID = -2524427972418434522L;
    private N root;

    public void clear() {
        root = null;
        size = 0;
    }

    public boolean isEmpty() {
        return size == 0;
    }

    public N getRoot() {
        return root;
    }

    protected void setRoot(N value) {
        this.root = value;
    }

}

This is the code i run that gives problems note that sometimes it works and sometimes it doesnt:

AVLTree<Integer,Integer> avlt = new AVLTree<Integer,Integer>();
Random r = new Random();
for(int i = 1; i <= SIZE; i++){
    avlt.insert(i, r.nextInt(keySize));
}
int vdel = r.nextInt(keySize);
avlt.deleteAll(vdel);
assertEquals(false, avlt.containsValue(vdel));

Answer 1

The problem is that the tree structure changes whilst you are trying to find nodes to delete.

Consider this tree, from which we will be deleting the 'X's:

     2->X
    /    \
1->X      3->X
              \
               4->O

The delete operation starts at '2'. As the '2' node has a left hand branch, we descend to the '1' node. The '1' node has neither left nor right branches so there is no further descent.

Node '1' must be deleted.

2->X
    \
     3->X
         \
          4->O

And then the tree needs rebalancing.

     3->X
   /     \
2->X     4->O

This completes the processing of the '1' node, so the recursive search moves back to '2'.

The left hand branch has been processed. Due to the rebalancing, there is no longer a right hand branch to process. The '2' node itself needs deleting.

3->X
    \
     4->O

This completes the processign of node '2' so we return. This completes the algorithm.

Nodes '3' and '4' were never inspected because of the rebalancing.

How to fix this?

The easiest solution is to separate the search from the "delete and rebalance" operation. Create a collection of all the keys that map to the value to be deleted. Once all such keys are identified, delete those mappings one at a time. As the tree does not change shape during the search, the search will find all the relevant key-value pairs.

My own preference would be to create an Iterator over the tree nodes that supports remove. Such an Iterator would be useful for tasks beyond deleting matching values. For such an iterator one needs to be able to identify the next node from any specified node. This means a node has to know what its parent is.

To that end, I suggest amending your code as follows:

The node implementation tracks its parent and its key cannot be changed.

  public static class AVLTreeNode<K extends Comparable<K>, V> implements Node<V> {

    private AVLTreeNode<K,V> left, right, parent;
    private final K key;
    private V value;
    private int height = 0;

    public AVLTreeNode(K key, V value){
      this.key = key;
      this.value = value;
    }

    protected void setLeft(AVLTreeNode<K,V> left) {
      this.left = left;
      if( left!=null ) {
        left.parent = this;
      }
    }

    public AVLTreeNode<K,V> getNext() {
      AVLTreeNode<K,V> next;

      // If a right branch exists, the next node is the left-most node in the right branch.
      if( right!=null ) {
        next = right;
        while( next.left!=null ) {
          next = next.left;
        }
        return next;
      }

      // The closest parent of which this is a left-branch descendant is the next node
      next = this;
      while( next.parent!=null ) {
        if( next.parent.left==next ) {
          return next.parent;
        }
        next = next.parent;
      }

      // no next node
      return null;
    }


    protected void setRight(AVLTreeNode<K,V> right) {
      this.right = right;
      if( right!=null ) {
        right.parent = this;
      }
    }


    ... rest of class is unchanged
  }

The tree's insert and delete method can move a node to the root of the tree, so they also have to set the node's parent to null if that happens.

  public void insert(K key, V value) {
    size++;
    AVLTreeNode<K,V> newRoot = insert(super.getRoot(),key,value);
    newRoot.parent = null;
    super.setRoot(newRoot);
  }

  public void delete(K key){
    AVLTreeNode<K,V> newRoot = delete(super.getRoot(), key);
    if( newRoot!=null ) {
      newRoot.parent = null;
    }
    super.setRoot(newRoot);
  }

The delete method changed the key of a node, moving it out of sequence. To preserve the sequence it cannot do that.

  private AVLTreeNode<K,V> delete(AVLTreeNode<K,V> root, K key) {
    if (root == null) {
      return root;
    } else if (root.getKey().compareTo(key) > 0) {
      root.setLeft(delete(root.left, key));
    } else if (root.getKey().compareTo(key) < 0) {
      root.setRight(delete(root.right, key));
    } else {
      if (root.left == null || root.right == null) {
        root = (root.left == null) ? root.right : root.left;
      } else {
        // Promote the next node to replace root.
        AVLTreeNode<K,V> mostLeftChild = minValueNode(root.right);
        mostLeftChild.setRight(delete(root.right, mostLeftChild.key));
        mostLeftChild.setLeft(root.left);
        root = mostLeftChild;
      }
      if(size > 0) size--;
    }
    if (root != null) {
      root = rebalance(root);
    }
    return root;
  }

Then we can create an Iterator:

  public static class AVLNodeIterator<K extends Comparable<K>,V> implements Iterator<AVLTreeNode<K,V>> {
    AVLTree<K,V> tree;
    AVLTreeNode<K,V> current;
    AVLTreeNode<K,V> next;

    public AVLNodeIterator(AVLTree<K,V> tree) {
      this.tree = tree;
      current = null;
      next = tree.minValueNode(tree.getRoot());
    }


    @Override
    public boolean hasNext() {
      return next!=null;
    }


    @Override
    public AVLTreeNode<K, V> next() {
      if( next==null ) {
        throw new NoSuchElementException();
      }
      current = next;
      next = current.getNext();
      return current;
    }


    @Override
    public void remove() {
      if( current==null ) {
        throw new IllegalStateException();
      }
      tree.delete(current.getKey());
      current=null;
    }

  }

and finally we can rewrite deleteAll to use the new iterator:

  public void deleteAll(V value){
    Iterator<AVLTreeNode<K,V>> iterator = new AVLNodeIterator<>(this);
    while( iterator.hasNext()) {
      AVLTreeNode<K,V> node = iterator.next();
      if( node.getValue().equals(value) ) {
        iterator.remove();
      }
    }
  }