Skip to content

feat(data-structures): add BinarySearchTree methods ceiling, floor, higher, lower #6544

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

WWRS
Copy link
Contributor

@WWRS WWRS commented Mar 31, 2025

closes #3069

May break custom binary trees that reimplement BinarySearchTree.#findNode(). I think it's unlikely that anyone is doing that given that #findNode() should work on all child data structures of binary search trees. As pointed out, this is not a problem.

The new methods have the same time complexity as find().

@WWRS WWRS requested a review from kt3k as a code owner March 31, 2025 17:04
Copy link

codecov bot commented Mar 31, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.30%. Comparing base (61f9fd3) to head (abbbf61).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6544   +/-   ##
=======================================
  Coverage   95.29%   95.30%           
=======================================
  Files         576      576           
  Lines       43315    43354   +39     
  Branches     6477     6489   +12     
=======================================
+ Hits        41279    41318   +39     
  Misses       1995     1995           
  Partials       41       41           

☔ 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.

@lionel-rowe
Copy link
Contributor

May break custom binary trees that reimplement BinarySearchTree.#findNode()

It's not possible to reimplement #private methods in derived classes. Implementing a method with the same name in the derived class creates a new method that's only accessible within the derived class, rather than overriding the method in the parent.

class A {
    #private() {
        return 'A'
    }

    foo() {
        console.log(this.#private())
    }
}

class B extends A {
    #private() {
        return 'B'
    }

    bar() {
        console.log(this.#private())
    }
}

new A().foo() // logs 'A'
new B().foo() // still logs 'A'
new B().bar() // logs 'B'

@WWRS
Copy link
Contributor Author

WWRS commented Apr 1, 2025

Ah, I see where I got confused. There's a change to an export in _binary_search_tree_internals and that gets used by RedBlackTree. I thought this meant RedBlackTree used some sort of extension of internals, but that's just exported to allow RedBlackTree access to those private methods. So the type change cannot cause any problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add method "higher" "lower" "floor" "ceiling" to BinarySearchTree
2 participants