feat(osmoutils): replace index-based loop with range in ContainsDupli… #9453
Conversation
WalkthroughThe Changes
Sequence Diagram(s)No sequence diagram generated as the change is a simple internal refactor without control flow modification. ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
osmoutils/slice_helper.go(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Summary
| visited := make(map[any]bool, 0) | ||
| for i := 0; i < len(arr); i++ { | ||
| if visited[arr[i]] { | ||
| for _, item := range arr { | ||
| if visited[item] { | ||
| return true | ||
| } else { | ||
| visited[arr[i]] = true | ||
| visited[item] = true | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Constrain T to comparable and switch to map[T]struct{} for safer, leaner duplicate detection
Using any permits non-comparable types and will fail to compile when the caller passes slices, maps, etc.
Additionally, storing bool values wastes a byte per entry and the else branch is unnecessary.
-func ContainsDuplicate[T any](arr []T) bool {
- visited := make(map[any]bool, 0)
+func ContainsDuplicate[T comparable](arr []T) bool {
+ visited := make(map[T]struct{})
...
- if visited[item] {
+ if _, ok := visited[item]; ok {
return true
- } else {
- visited[item] = true
+ }
+ visited[item] = struct{}{}This keeps the public behaviour intact while tightening the API contract, trimming memory, and eliminating the redundant else.
🤖 Prompt for AI Agents
In osmoutils/slice_helper.go around lines 44 to 50, constrain the generic type T
to the comparable interface to ensure only comparable types are used as map
keys, preventing compilation errors with non-comparable types. Replace the map
key type from any to T and change the map value type from bool to struct{} to
reduce memory usage. Remove the unnecessary else branch by directly setting the
map entry after the duplicate check. This will tighten the API contract,
optimize memory, and simplify the code.
|
This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed if no further activity occurs. Thank you! |
Replace the classic index-based for loop with the more idiomatic Go range loop in the ContainsDuplicate function. This change improves code readability, follows Go best practices, and reduces the potential for index-related errors while maintaining identical functionality