feat: add callback functionality for binary search tree

[dquixote-jr]

I need the functionality to find the k'th node in the tree, which isn't possible with current setup
This augmentation allows people to define whatever functions that they want, in this particular case, it allows me to store the subtree size in a parameter st_size and which I can use to find the k'th node in order.

First javascript project PR so this is gonna have some issues, happy to solve them.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[kt3k]

Can you use camelCase?

[dquixote-jr]

done.

[kt3k]

Need to document callback param in jsdoc.

[dquixote-jr]

Added the param.

[kt3k]

I need the functionality to find the k'th node in the tree, which isn't possible with current setup

This change doesn't seem a good idea for adding that capability to this class. Currently BinarySearchNode class is private. This makes it a part of public API. That doesn't seem an ideal change. Can we achieve this without exposing node class public?

For small k's, it seems we can use lnrValues (or rnlValues) iterator for finding k'th node. What do your tree size and k value typically look like?

[dquixote-jr]

For small k's, it seems we can use lnrValues (or rnlValues) iterator for finding k'th node. What do your tree size and k value typically look like?

The tree can be large, like ~5K. And k can be as large as O(n), so iterating over values with lnr/rnl isn't really a good idea.

This change doesn't seem a good idea for adding that capability to this class. Currently BinarySearchNode class is private.

I don't think it's the worst thing to make the BinarySearchNode a public class, as we have already made it a "binary" tree, so having left/right/parent elements is expected of it. A different thing to do would be to expose the BinarySearchNode class, but not give access directly to member objects but rather have setChild/getChild or set/get Left/Right/Parent kind of functions.
I'm open to any other ideas too, my main use case is to allow people to travel over the tree via nodes somehow via left/right nodes. I don't need them to be able to access the specific internal attributes of the class, just left/right children.

[codecov]

Codecov Report

Attention: Patch coverage is 91.66667% with 6 lines in your changes missing coverage. Please review.

Project coverage is 93.70%. Comparing base (2258bf2) to head (a2fd74c).

Files with missing lines	Patch %	Lines
data_structures/binary_search_tree.ts	91.17%	2 Missing and 1 partial ⚠️
data_structures/red_black_tree.ts	66.66%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6730      +/-   ##
==========================================
- Coverage   94.65%   93.70%   -0.95%     
==========================================
  Files         576      559      -17     
  Lines       47065    46838     -227     
  Branches     6610     6472     -138     
==========================================
- Hits        44549    43890     -659     
- Misses       2473     2902     +429     
- Partials       43       46       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[kt3k]

I don't think it's the worst thing to make the BinarySearchNode a public class, as we have already made it a "binary" tree, so having left/right/parent elements is expected of it. A different thing to do would be to expose the BinarySearchNode class, but not give access directly to member objects but rather have setChild/getChild or set/get Left/Right/Parent kind of functions.

Then maybe it's better to define a new interface which only has left, right, and parent (and maybe value) fields, and use it as input type for callback?

Also run deno task lint and see the output. We need to fix them to merge this to main

[dquixote-jr]

Then maybe it's better to define a new interface which only has left, right, and parent (and maybe value) fields, and use it as input type for callback?

That sounds like a good idea. I've added a new class BSTNode and refactored the setup to use that.

Also run deno task lint and see the output. We need to fix them to merge this to main

Thank you for this. I've got it passing locally. import type gave me a headspin but I got it working in the end :)

[kt3k]

This now looks much better to me. Left some comments

[kt3k]

We usually avoid using this type of acronym in API names. BinarySearchTree used to be BSTree but we renamed it later #2400 I think this should be named BinarySearchTreeNode.

Also I think this should be typescript interface instead of class. We can avoid unnecessary runtime overhead by making this interface

[dquixote-jr]

OK, renamed to BinarySearchTreeNode and made into an interface.

[kt3k]

Nicely documented 👍