feat: AVL Implementation of BinarySearchTree

[ngtrhieu]

Description:

Written a simple AVL Tree extending BinarySearchTree. Expose BST insertImpl and deleteImpl for a more elegant AVL extension :)

Essentially:

_insertImpl(value, node) {
    node = super._insertImpl(value, node);
    return this._balance(node);
}

_deleteImpl(value, node) {
    node = super._deleteImpl(value, node);
    return this._balance(node);
}

Type of change:

New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Tested against the test cases for BST.
Added extra test cases for AVL rotations to check for correctness.

Current problem:

This AVL implementations should work flawlessly for tree with unique elements, but would not ensure correctness when we have duplicate elements (due to rotations, check the skipped tests).

I kinda think of 3 ways to deal with it:

• Make AVL implementation separate from BST -- useful if you want to reference how to write AVL from scratch :), but lose the elegant-ness of having an AVL extension.
• Change BST implementation such as a BST node keep track of how many duplicates it have. the BST definition can use strict lesser/greater.
• Change BST implementation to use a 'comparer/comparator'. AVL can then "mod" the comparer to for tiebreak, essentially eliminate duplicates.

Your choice on how you want to deal with this :)

Related Issues

#5

[yangshun]

Thanks a lot Hieu!

[yangshun]

Thanks for catching this

[yangshun]

I think I'll leave out the case of handling duplicates. Do you have any recommendations on how this is done usually?

[ngtrhieu]

No particular preferences actually, I tend to follow the patterns employed in the framework/project:

For example, Java/C# libraries heavily relied on template method pattern (they use Comparer/Comparator) to alter the behaviour of the algorithm. So if I were to work on these languages, I probably modify the Comparer to handle tiebreak. As long as your comparator is deterministic you are ok:

// Assume GetObjectId returns an unique id. In C/C++ maybe you can  rely on pointer memory address. 
const tiebreakComparer = (a,b) => comparer(a,b) || a.GetObjectId().CompareTo(b.GetObjectId())

Having a comparer makes your data struct really versatile 😄

However, the proper AVL tree implementation would normally handle duplicates by keeping an extra integer in the node for duplication count. i.e:

class AvlTreeNode extends BinaryTreeNode {
     constructor AvlTreeNode(value) {
           super(value);
           this.duplicate = 1;
     }
}

For insert/delete, when binarySearch the exact item, just +1 on duplicate for insert and -1 for delete. Only to the actual delete when duplicate === 0

[ngtrhieu]

Personally for this project, I think it is best to include the comparer for BinarySearchTree for two reasons:

• Including duplicate handling is not a "textbook" solution for a textbook BST problem, considering this repo is for learning purposes.
• Comparer doesn't affect your code readability.

Something like this:

class BinarySearchTree {
  constructor(comparer) {
    this.root = null;
    this.comparer = comparer || ((a, b) => a - b);
  }
  ...
}

I have the whole class already implemented in Js. Would you like me to make the same modification for Ts?