✨ feat(mq-lang): Add bsearch function and tests#1353
Conversation
- Implements binary search (bsearch) for sorted arrays in builtin.mq - Adds comprehensive tests for bsearch in builtin_tests.mq Closes #1341
There was a problem hiding this comment.
Pull request overview
This PR implements a bsearch (binary search) function in mq's built-in library, as requested in issue #1341. The function searches a sorted array for a target value and returns its index, or -1 if not found. The implementation uses a recursive inner lambda (_bsearch_impl) following the same idiom as the existing find_index function.
Changes:
- Adds the
bsearchfunction inbuiltin.mqwith input validation (must be an array) and a recursive inner lambda for the binary search logic - Adds
test_bserchtest function inbuiltin_tests.mqcovering found/not-found and various element types
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
crates/mq-lang/builtin.mq |
Implements bsearch(arr, target) using a recursive inner lambda _bsearch_impl |
crates/mq-lang/builtin_tests.mq |
Adds test function for bsearch with 4 test cases |
crates/mq-lang/builtin_tests.mq
Outdated
| | assert_eq(result3, "abc ") | ||
| end | ||
|
|
||
| def test_bserch(): |
There was a problem hiding this comment.
The test function is named test_bserch, which is a misspelling of "search". It should be named test_bsearch to match the function being tested.
| def test_bserch(): | |
| def test_bsearch(): |
crates/mq-lang/builtin_tests.mq
Outdated
| def test_bserch(): | ||
| let result1 = bsearch([1, 2, 3, 4, 5], 3) | ||
| | assert_eq(result1, 2) | ||
|
|
||
| | let result2 = bsearch([1, 2, 3, 4, 5], 6) | ||
| | assert_eq(result2, -1) | ||
|
|
||
| | let result3 = bsearch([10, 20, 30], 10) | ||
| | assert_eq(result3, 0) | ||
|
|
||
| | let result4 = bsearch(["a", "b", "c"], "b") | ||
| | assert_eq(result4, 1) | ||
| end |
There was a problem hiding this comment.
The test function test_bserch (and after fixing the typo, test_bsearch) is defined but never registered in the run_tests([...]) call. Without this registration, the tests will never be executed as part of the test suite. A corresponding test_case("bsearch", test_bsearch) entry should be added, ideally in the "# Array manipulation" section alongside similar array functions like find_index.
| | assert_eq(result3, 0) | ||
|
|
||
| | let result4 = bsearch(["a", "b", "c"], "b") | ||
| | assert_eq(result4, 1) |
There was a problem hiding this comment.
The test suite is missing a test case for an empty array input: bsearch([], x). When an empty array is passed, len([]) - 1 evaluates to -1, so _bsearch_impl is called with low = 0 and high = -1, which should correctly return -1. This edge case is worth covering explicitly in the tests, both to document the expected behavior and to guard against regressions.
| | assert_eq(result4, 1) | |
| | assert_eq(result4, 1) | |
| | let result5 = bsearch([], 1) | |
| | assert_eq(result5, -1) |
Co-authored-by: harehare <533078+harehare@users.noreply.github.com>
Fix typo and register bsearch test in run_tests
Closes #1341