-
-
Notifications
You must be signed in to change notification settings - Fork 736
Add solution for Challenge 21 by yz4230 #747
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
Conversation
WalkthroughA new Go submission file implementing three binary search utility functions: an iterative binary search, a recursive variant, and a function to find insertion position in a sorted array. The file includes a main function demonstrating usage of these utilities. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 1
🧹 Nitpick comments (2)
challenge-21/submissions/yz4230/solution-template.go (2)
7-24: Consider adding more comprehensive test cases.The main function demonstrates basic usage well, but for a practice submission, consider testing additional scenarios:
- Target not found (for both search functions)
- Empty array
- Single element array
- Target at boundaries (first/last element)
- Target outside array range (smaller/larger than all elements)
64-78: Significant code duplication with BinarySearch.Lines 67-75 duplicate the search logic from
BinarySearch(lines 31-39). The only difference is the final return value (leftvs-1).Consider refactoring to eliminate duplication, for example by having
BinarySearchuseFindInsertPositioninternally:func BinarySearch(arr []int, target int) int { pos := FindInsertPosition(arr, target) if pos < len(arr) && arr[pos] == target { return pos } return -1 }Or by extracting the common binary search logic into a helper function.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
challenge-21/submissions/yz4230/solution-template.go(1 hunks)
🔇 Additional comments (1)
challenge-21/submissions/yz4230/solution-template.go (1)
46-60: LGTM! Correct recursive implementation.The recursive binary search correctly implements a closed interval
[left, right]approach with proper base case handling and boundary adjustments.
| func BinarySearch(arr []int, target int) int { | ||
| left, right := 0, len(arr) | ||
| for left < right { | ||
| mid := (right - left) / 2 + left | ||
| midv := arr[mid] | ||
| if target < midv { | ||
| right = mid | ||
| } else if midv < target { | ||
| left = mid + 1 | ||
| } else { | ||
| return mid | ||
| } | ||
| } | ||
| return -1 | ||
| } |
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.
Inconsistent interval convention with recursive variant.
The iterative BinarySearch uses a half-open interval [left, right) (initialized with right := len(arr)), while BinarySearchRecursive uses a closed interval [left, right] (called with len(arr)-1). Both implementations are correct individually, but the inconsistency can confuse maintainers.
Consider standardizing on one convention across both functions for better code consistency.
🤖 Prompt for AI Agents
challenge-21/submissions/yz4230/solution-template.go lines 28-42: the iterative
BinarySearch uses a half-open interval [left,right) (right := len(arr)) while
the recursive variant uses a closed interval [left,right] (invoked with
len(arr)-1), causing inconsistency; pick one convention and make both functions
match — either change the iterative function to use closed interval (initialize
right := len(arr)-1, adjust loop condition to left <= right and update pointers
accordingly) or change the recursive variant to use half-open bounds (accept
right as len(arr) and treat right as exclusive in the recursion), and update all
comparisons/indices and base cases to follow the chosen convention consistently.
Challenge 21 Solution
Submitted by: @yz4230
Challenge: Challenge 21
Description
This PR contains my solution for Challenge 21.
Changes
challenge-21/submissions/yz4230/solution-template.goTesting
Thank you for reviewing my submission! 🚀