-
-
Notifications
You must be signed in to change notification settings - Fork 752
Add solution for Challenge 21 by cckwes #849
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 program is introduced with three binary search utility functions: an iterative implementation, a recursive implementation, and a function to find insertion positions in sorted arrays. Each function handles edge cases and returns -1 or 0 when targets are not found. 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: 0
🧹 Nitpick comments (2)
challenge-21/submissions/cckwes/solution-template.go (2)
54-77: Consider simplifying the recursive API and base casesThe recursive search logic is correct, but you could simplify and make the API harder to misuse:
- The
len(arr) == 0check is redundant if callers respectleft/right;left > rightis already a sufficient base case.- Exposing
leftandrighton the exported function makes it easier for external callers to pass out‑of‑range values and trigger a panic.If the challenge doesn’t require this exact signature, you could wrap it like:
// BinarySearchRecursive is the exported entry point. func BinarySearchRecursive(arr []int, target int) int { return binarySearchRecursive(arr, target, 0, len(arr)-1) } func binarySearchRecursive(arr []int, target, left, right int) int { if left > right { return -1 } mid := (left + right) / 2 val := arr[mid] if val == target { return mid } if val > target { return binarySearchRecursive(arr, target, left, mid-1) } return binarySearchRecursive(arr, target, mid+1, right) }If the signature is fixed by the template, you can at least drop the
len(arr) == 0check to reduce work in each recursive call.
79-105: ClarifyFindInsertPositionsemantics for duplicates (optionally switch to strict lower bound)For arrays without duplicates, this works as expected: returns the index of
targetif present, otherwise the position where it should be inserted to keep the slice sorted.With duplicates, though, the index returned for an existing
targetdepends on how the mid converges (e.g., for[2,2,2,2]andtarget=2, it returns1, not0). If the intended contract is “lower bound” (first index wherearr[i] >= target), you can tweak it slightly:func FindInsertPosition(arr []int, target int) int { - if len(arr) == 0 { - return 0 - } - - left := 0 - right := len(arr) - 1 - - for left <= right { - index := (left + right) / 2 - val := arr[index] - - if val == target { - return index - } - - if val < target { - left = index + 1 - } else { - right = index - 1 - } - } - - return left + left, right := 0, len(arr) + for left < right { + mid := (left + right) / 2 + if arr[mid] < target { + left = mid + 1 + } else { + right = mid + } + } + return left }This keeps the same behavior for unique elements but guarantees a stable “insert before all existing equals” position if duplicates are present. If the challenge never uses duplicates, what you have is still logically sound.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
challenge-21/submissions/cckwes/solution-template.go(1 hunks)
🔇 Additional comments (2)
challenge-21/submissions/cckwes/solution-template.go (2)
7-24: Main function is a clear, self-contained demoThe
mainfunction cleanly demonstrates all three helpers with simple sample data and doesn’t interfere with their use in tests. No changes needed here.
26-52: IterativeBinarySearchimplementation looks correct and robustThe iterative binary search correctly initializes bounds, handles the empty-slice case, and updates
left/rightwithout off‑by‑one issues. Behavior (-1when not found) is clear and conventional.
Challenge 21 Solution
Submitted by: @cckwes
Challenge: Challenge 21
Description
This PR contains my solution for Challenge 21.
Changes
challenge-21/submissions/cckwes/solution-template.goTesting
Thank you for reviewing my submission! 🚀