Add solution for Challenge 21 by rodney-b #707
Conversation
WalkthroughAdds a new Go solution file implementing three exported functions: an iterative binary search, a recursive binary search, and a function to compute the sorted insertion index; includes a demo Changes
Sequence Diagram(s)sequenceDiagram
participant Main as main()
participant Iter as BinarySearch
participant Rec as BinarySearchRecursive
participant Ins as FindInsertPosition
participant Arr as []int (sorted)
rect rgba(50,115,220,0.08)
Main->>Iter: call BinarySearch(arr, target)
Iter->>Arr: probe mid index
Arr-->>Iter: value
Iter-->>Main: index or -1
end
rect rgba(50,115,220,0.08)
Main->>Rec: call BinarySearchRecursive(arr, target, left, right)
Rec->>Arr: probe mid (recursive)
Arr-->>Rec: value
Rec-->>Main: index or -1
end
rect rgba(50,115,220,0.08)
Main->>Ins: call FindInsertPosition(arr, target)
Ins->>Arr: binary probe to find insert idx
Arr-->>Ins: values
Ins-->>Main: insert index
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Pre-merge checks❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
challenge-21/submissions/rodney-b/solution-template.go (3)
52-71: Consider simplifying the switch cases.The function works correctly, but contains some redundant code:
- Line 54: The
arrLen == 0check is redundant sinceleft > rightalready handles empty arrays when called with valid bounds- Lines 66-69: The explicit
case target == val:and thedefaultcase are redundant. Since the first two cases covertarget < valandtarget > val, the remaining case must betarget == val, so you can usedefault: return middirectlyApply this diff to simplify:
func BinarySearchRecursive(arr []int, target int, left int, right int) int { - arrLen := len(arr) - if left > right || arrLen == 0 { + if left > right { return -1 } mid := left + (right-left)/2 val := arr[mid] switch { case target < val: return BinarySearchRecursive(arr, target, left, mid-1) case target > val: return BinarySearchRecursive(arr, target, mid+1, right) - case target == val: + default: return mid - default: - return -1 } }
84-84: Simplify redundant loop condition.The condition
right >= left && left <= rightis redundant since both comparisons express the same relationship. Use just one.Apply this diff:
- for right >= left && left <= right { + for left <= right {
93-94: Clarify behavior when target exists.The comment "or mid + 1" suggests uncertainty about whether to return the index of an existing element or the position after it. For standard insertion position semantics (maintaining sorted order while allowing duplicates), returning
midwhen the target exists is correct and places the new element before the existing one.If this behavior is intentional, consider removing the ambiguous comment or clarifying the intended semantics:
- return mid // or mid + 1 + return mid // Insert before existing element
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
challenge-21/submissions/rodney-b/solution-template.go(1 hunks)
🔇 Additional comments (2)
challenge-21/submissions/rodney-b/solution-template.go (2)
7-24: LGTM! Good demo coverage.The main function effectively demonstrates all three binary search implementations with clear output.
28-48: LGTM! Solid iterative implementation.The overflow-safe midpoint calculation and clear switch-case logic make this a robust binary search implementation.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
challenge-21/submissions/rodney-b/solution-template.go (2)
53-69: Consider wrapping the recursive helper for a cleaner API.The recursive implementation is correct. However, exposing
leftandrightparameters requires callers to understand implementation details (they must pass0andlen(arr)-1).For a cleaner API, consider this pattern:
-// BinarySearchRecursive performs binary search using recursion. -// Returns the index of the target if found, or -1 if not found. -func BinarySearchRecursive(arr []int, target int, left int, right int) int { +// BinarySearchRecursive performs binary search using recursion. +// Returns the index of the target if found, or -1 if not found. +func BinarySearchRecursive(arr []int, target int) int { + return binarySearchRecursiveHelper(arr, target, 0, len(arr)-1) +} + +func binarySearchRecursiveHelper(arr []int, target int, left int, right int) int { if left > right { return -1 } mid := left + (right-left)/2 val := arr[mid] switch { case target < val: - return BinarySearchRecursive(arr, target, left, mid-1) + return binarySearchRecursiveHelper(arr, target, left, mid-1) case target > val: - return BinarySearchRecursive(arr, target, mid+1, right) + return binarySearchRecursiveHelper(arr, target, mid+1, right) default: return mid } }This would also simplify the call in
main:- recursiveIndex := BinarySearchRecursive(arr, target, 0, len(arr)-1) + recursiveIndex := BinarySearchRecursive(arr, target)
73-97: LGTM! Consider usingdefaultfor consistency.The insertion position logic is correct and handles all edge cases properly including empty arrays.
For consistency with
BinarySearchandBinarySearchRecursive, consider usingdefaultinstead of the explicit equality case:switch { case target < val: right = mid - 1 case target > val: left = mid + 1 - case target == val: + default: return mid // Insert before existing element }Both approaches are correct; this suggestion is purely for stylistic consistency across the three functions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
challenge-21/submissions/rodney-b/solution-template.go(1 hunks)
🔇 Additional comments (2)
challenge-21/submissions/rodney-b/solution-template.go (2)
7-24: LGTM!The demo clearly exercises all three functions with appropriate test cases.
28-49: LGTM!The iterative binary search is correctly implemented with proper overflow prevention and handles all edge cases including empty arrays.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
challenge-21/submissions/rodney-b/solution-template.go (2)
53-69: LGTM!The recursive implementation is correct and handles the base case properly.
For production code, you might consider wrapping this with a simpler public API that doesn't expose
leftandright:func BinarySearchRecursive(arr []int, target int) int { return binarySearchRecursive(arr, target, 0, len(arr)-1) } func binarySearchRecursive(arr []int, target int, left int, right int) int { // existing implementation }However, for a coding challenge, the current approach is perfectly acceptable.
71-97: Clarify behavior when target already exists.The implementation is correct, but the function documentation could be clearer about what happens when the target already exists in the array. Currently, when
target == val(line 91), you returnmid, which means inserting at that index would place the new element before the existing one.Consider enhancing the function comment:
-// FindInsertPosition returns the index where the target should be inserted -// to maintain the sorted order of the array. +// FindInsertPosition returns the index where the target should be inserted +// to maintain the sorted order of the array. +// If the target already exists, returns the index of the existing element +// (inserting at this index would place the new element before the existing one).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
challenge-21/submissions/rodney-b/solution-template.go(1 hunks)
🔇 Additional comments (2)
challenge-21/submissions/rodney-b/solution-template.go (2)
7-24: LGTM!The demo code effectively demonstrates all three functions with clear output formatting.
28-49: LGTM!The iterative binary search implementation is correct and uses overflow-safe mid-point calculation. The switch statement provides clean, idiomatic Go code.
3 participants
No description provided.