GeezerVison is showing github.com — fix(searches): fix boundary safety and null checks in SaddlebackSearch by hardikkaurani · Pull Request #7726 · TheAlgorithms/Java
Conversation
- I have read CONTRIBUTING.md.
- This pull request is all my own work -- I have not plagiarized it.
- All filenames are in PascalCase.
- All functions and variable names follow Java naming conventions.
- All new algorithms have a URL in their comments that points to Wikipedia or other similar explanations.
- All new algorithms include a corresponding test class that validates their functionality.
- All new code is formatted with
clang-format -i --style=file path/to/your/file.java
Description
This pull request fixes boundary safety, null handling, visibility, and adds a convenience overload in SaddlebackSearch:
- Row & Column Boundary Safety: In
find(int[][] arr, int row, int col, int key), the check previously only tested row < 0 || col >= arr[row].length. Passing row >= arr.length would trigger an ArrayIndexOutOfBoundsException when evaluating arr[row]. Furthermore, negative col values were not guarded against and would trigger ArrayIndexOutOfBoundsException on arr[row][col]. Added row >= arr.length, col < 0, and arr[row] == null guards before accessing row contents.
- Null Array Handling: Updated empty check from
if (arr.length == 0) to if (arr == null || arr.length == 0) to avoid NullPointerException and consistently throw IllegalArgumentException.
- Method Visibility: Changed method modifier from package-private
static int[] find to public static int[] find.
- Convenience Entrypoint: Added overloaded
public static int[] find(int[][] arr, int key) that initiates search from the bottom-left corner (find(arr, arr.length - 1, 0, key)).
- Comprehensive Tests: Added unit tests in
SaddlebackSearchTest for the convenience method, null arrays, out-of-bounds rows, negative column indices, and matrix with null rows.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Hi @DenizAltunkapan,
Could you please review this pull request when you have a moment?
Summary of Changes
- Boundary Safety: Added guards in
SaddlebackSearch.find() against row >= arr.length, negative col values, and null rows to prevent ArrayIndexOutOfBoundsException and NullPointerException.
- Null Input Validation: Added a null check on
arr to consistently throw IllegalArgumentException when a null or empty array is provided.
- API Improvements: Made the
find methods public static and introduced a convenience overload, find(int[][] arr, int key), that automatically starts the search from the bottom-left corner.
- Test Coverage: Added unit tests covering out-of-bounds indices, null arrays, null rows, and the convenience overload.
Test Results
SaddlebackSearchTest: 12 passed, 0 failed, 0 skipped (BUILD SUCCESS).
- Formatted the changes using
clang-format in accordance with repository guidelines.
Thank you for your time and consideration!
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
❌ Patch coverage is 60.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.22%. Comparing base (8b49bc5) to head (c2d1dda).
Additional details and impacted files
@@ Coverage Diff @@
## master #7726 +/- ##
============================================
- Coverage 82.23% 82.22% -0.01%
- Complexity 8402 8409 +7
============================================
Files 843 843
Lines 26140 26143 +3
Branches 5100 5100
============================================
Hits 21495 21495
- Misses 3853 3854 +1
- Partials 792 794 +2
☔ View full report in Codecov by Harness.
📢 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.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Reviewers
DenizAltunkapan
Awaiting requested review from DenizAltunkapan
DenizAltunkapan is a code owner
yanglbme
Awaiting requested review from yanglbme
yanglbme is a code owner
alxkm
Awaiting requested review from alxkm
alxkm is a code owner
At least 1 approving review is required to merge this pull request.
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
|