Add solution for Challenge 27 by Johrespi #723
Conversation
WalkthroughAdds a Go generics utilities module providing Pair, Stack, Queue, Set types plus common slice utilities and a shared ErrEmptyCollection error for empty-collection operations (constructors, accessors, mutators, and set operations included). Changes
Sequence Diagram(s)sequenceDiagram autonumber participant C as Client participant G as generics package rect rgb(240,248,255) note over C,G: Create and operate on collections C->>G: NewStack[T]() C->>G: Push(value) C->>G: Pop() alt empty G-->>C: ErrEmptyCollection else not empty G-->>C: value end end rect rgb(245,255,250) C->>G: NewQueue[T]() C->>G: Enqueue(value) C->>G: Dequeue() alt empty G-->>C: ErrEmptyCollection else not empty G-->>C: value end end rect rgb(255,250,240) C->>G: NewSet[T]() C->>G: Add(v) C->>G: Union(s1,s2) G-->>C: *Set[T] end Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Pre-merge checks✅ Passed checks (3 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: 1
🧹 Nitpick comments (1)
challenge-27/submissions/Johrespi/solution-template.go (1)
19-213: Clean up placeholder TODO comments.The functions are already implemented, so the remaining
// TODOmarkers are misleading. Please remove them (and similar ones throughout the file) to avoid implying unfinished work.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (1)
challenge-27/submissions/Johrespi/solution-template.go (1)
245-258: Handle nil set inputs.The
Intersectionfunction will panic if eithers1ors2is a zero-value Set with nil maps. Add nil checks before ranging.func Intersection[T comparable](s1, s2 *Set[T]) *Set[T] { -// TODO: Implement this function - s3 := NewSet[T]() -for k, _ := range s1.items { -if s2.Contains(k) { -s3.Add(k) +if s1 != nil && s1.items != nil && s2 != nil { +for k := range s1.items { +if s2.Contains(k) { +s3.Add(k) +} } } return s3 }
🧹 Nitpick comments (4)
challenge-27/submissions/Johrespi/solution-template.go (4)
88-96: Simplify to idiomatic Go.The IsEmpty method can be simplified to a single return statement.
func (s *Stack[T]) IsEmpty() bool { -// TODO: Implement this method - -if len(s.items) == 0 { -return true -} - -return false +return len(s.items) == 0 }
157-163: Simplify to idiomatic Go.The IsEmpty method can be simplified to a single return statement.
func (q *Queue[T]) IsEmpty() bool { -// TODO: Implement this method -if len(q.items) == 0 { -return true -} -return false +return len(q.items) == 0 }
206-210: Guard zero-value Set from panics.Calling
Sizeon a zero-value Set returns 0 without panicking (sincelen(nil)is 0), but for defensive programming and consistency with other methods, consider adding an explicit nil check.func (s *Set[T]) Size() int { -// TODO: Implement this method +if s.items == nil { +return 0 +} return len(s.items) }
339-355: Simplify boolean map check.The comparison at line 347 is verbose. In Go, map lookups return false for missing keys, so you can check the boolean value directly.
for _, e := range slice { -if m[e] == true { +if m[e] { continue } res = append(res, e) m[e] = true }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
challenge-27/submissions/Johrespi/solution-template.go(1 hunks)
🔇 Additional comments (10)
challenge-27/submissions/Johrespi/solution-template.go (10)
5-6: LGTM!The sentinel error pattern is correctly implemented and provides clear semantics for empty collection operations.
12-28: LGTM!The Pair implementation is correct with proper generic type handling and the Swap method correctly inverts the type parameters.
34-79: LGTM!The Stack implementation correctly handles LIFO semantics with proper empty-collection error handling.
181-188: LGTM!The nil check for zero-value Set correctly addresses the previous review feedback.
278-289: LGTM!The Filter implementation correctly applies the predicate and builds the result slice.
291-300: LGTM!The Map implementation correctly applies the mapper function to transform elements.
302-311: LGTM!The Reduce implementation correctly accumulates values using the reducer function.
313-324: LGTM!The Contains implementation correctly checks for element presence in the slice.
326-337: LGTM!The FindIndex implementation correctly returns the index of the first occurrence or -1 if not found.
212-226: Guard zero-value Set from panics.Calling
Elementson a zero-value Set panics when ranging overs.itemsat line 221. Please add a nil check.func (s *Set[T]) Elements() []T { -// TODO: Implement this method - -if len(s.items) == 0 { +if s.items == nil || len(s.items) == 0 { return []T{} }Likely an incorrect or invalid review comment.
| // Dequeue removes and returns the front element from the queue | ||
| // Returns an error if the queue is empty | ||
| func (q *Queue[T]) Dequeue() (T, error) { | ||
| // TODO: Implement this method | ||
| var zero T | ||
| | ||
| if len(q.items) == 0 { | ||
| return zero, ErrEmptyCollection | ||
| } | ||
| | ||
| removed := q.items[0] | ||
| q.items = q.items[1:] | ||
| | ||
| return removed, nil | ||
| } |
There was a problem hiding this comment.
Memory leak: backing array retains removed elements.
The slice reslicing at line 131 (q.items = q.items[1:]) doesn't release the memory of dequeued elements. The backing array still holds references to removed items, preventing garbage collection. For long-lived queues, this can accumulate substantial memory.
Apply this diff to nil out the removed element before reslicing:
removed := q.items[0] +q.items[0] = zero // Release reference for GC q.items = q.items[1:]Alternatively, for high-throughput queues, consider implementing a ring buffer or periodic slice reallocation when the queue becomes empty.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Dequeue removes and returns the front element from the queue | |
| // Returns an error if the queue is empty | |
| func (q *Queue[T]) Dequeue() (T, error) { | |
| // TODO: Implement this method | |
| var zero T | |
| if len(q.items) == 0 { | |
| return zero, ErrEmptyCollection | |
| } | |
| removed := q.items[0] | |
| q.items = q.items[1:] | |
| return removed, nil | |
| } | |
| // Dequeue removes and returns the front element from the queue | |
| // Returns an error if the queue is empty | |
| func (q *Queue[T]) Dequeue() (T, error) { | |
| // TODO: Implement this method | |
| var zero T | |
| if len(q.items) == 0 { | |
| return zero, ErrEmptyCollection | |
| } | |
| removed := q.items[0] | |
| q.items[0] = zero // Release reference for GC | |
| q.items = q.items[1:] | |
| return removed, nil | |
| } |
🤖 Prompt for AI Agents
In challenge-27/submissions/Johrespi/solution-template.go around lines 120 to 134, the Dequeue implementation reslices with q.items = q.items[1:] which leaves the removed element referenced in the backing array and prevents GC; before reslicing, assign the removed slot to the zero value (e.g., q.items[0] = zero) to drop the reference, then reslice; for high-throughput scenarios consider switching to a ring buffer or periodically copying remaining items into a new slice when the head index grows large. | // Remove removes an element from the set if it exists | ||
| func (s *Set[T]) Remove(value T) { | ||
| // TODO: Implement this method | ||
| delete(s.items, value) | ||
| } |
There was a problem hiding this comment.
Guard zero-value Set from panics.
Calling Remove on a zero-value Set panics because s.items is nil, similar to the issue fixed in Add. For consistency and safety, please initialize the map on demand.
func (s *Set[T]) Remove(value T) { -// TODO: Implement this method +if s.items == nil { +return +} delete(s.items, value) }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Remove removes an element from the set if it exists | |
| func (s *Set[T]) Remove(value T) { | |
| // TODO: Implement this method | |
| delete(s.items, value) | |
| } | |
| // Remove removes an element from the set if it exists | |
| func (s *Set[T]) Remove(value T) { | |
| if s.items == nil { | |
| return | |
| } | |
| delete(s.items, value) | |
| } |
🤖 Prompt for AI Agents
In challenge-27/submissions/Johrespi/solution-template.go around lines 190 to 194, calling Remove on a zero-value Set can panic because s.items may be nil; guard against this by checking if s.items == nil and initializing it with make(map[T]struct{}) before performing the delete so Remove becomes safe to call on zero-value sets. | // Contains returns true if the set contains the given element | ||
| func (s *Set[T]) Contains(value T) bool { | ||
| // TODO: Implement this method | ||
| _, exists := s.items[value] | ||
| if exists { | ||
| return true | ||
| } | ||
| return false | ||
| } |
There was a problem hiding this comment.
Guard zero-value Set from panics.
Calling Contains on a zero-value Set panics because s.items is nil. Please add a nil check and simplify the return logic.
func (s *Set[T]) Contains(value T) bool { -// TODO: Implement this method -_, exists := s.items[value] -if exists { -return true -} -return false +if s.items == nil { +return false +} +return s.items[value] }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Contains returns true if the set contains the given element | |
| func (s *Set[T]) Contains(value T) bool { | |
| // TODO: Implement this method | |
| _, exists := s.items[value] | |
| if exists { | |
| return true | |
| } | |
| return false | |
| } | |
| // Contains returns true if the set contains the given element | |
| func (s *Set[T]) Contains(value T) bool { | |
| if s.items == nil { | |
| return false | |
| } | |
| return s.items[value] | |
| } |
🤖 Prompt for AI Agents
In challenge-27/submissions/Johrespi/solution-template.go around lines 196 to 204, Contains panics when called on a zero-value Set because s.items can be nil; add a guard that returns false if s is nil or s.items is nil, and simplify the logic to directly return the lookup result (e.g., perform the nil checks then return exists). | // Union returns a new set containing all elements from both sets | ||
| func Union[T comparable](s1, s2 *Set[T]) *Set[T] { | ||
| // TODO: Implement this function | ||
| | ||
| s3 := NewSet[T]() | ||
| | ||
| for k, _ := range s1.items { | ||
| s3.Add(k) | ||
| } | ||
| | ||
| for k, _ := range s2.items { | ||
| s3.Add(k) | ||
| } | ||
| | ||
| return s3 | ||
| } |
There was a problem hiding this comment.
Handle nil set inputs.
The Union function will panic if either s1 or s2 is a zero-value Set with nil maps. Add nil checks before ranging over the maps.
func Union[T comparable](s1, s2 *Set[T]) *Set[T] { -// TODO: Implement this function - s3 := NewSet[T]() -for k, _ := range s1.items { -s3.Add(k) +if s1 != nil && s1.items != nil { +for k := range s1.items { +s3.Add(k) +} } -for k, _ := range s2.items { -s3.Add(k) +if s2 != nil && s2.items != nil { +for k := range s2.items { +s3.Add(k) +} } return s3 }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Union returns a new set containing all elements from both sets | |
| func Union[T comparable](s1, s2 *Set[T]) *Set[T] { | |
| // TODO: Implement this function | |
| s3 := NewSet[T]() | |
| for k, _ := range s1.items { | |
| s3.Add(k) | |
| } | |
| for k, _ := range s2.items { | |
| s3.Add(k) | |
| } | |
| return s3 | |
| } | |
| // Union returns a new set containing all elements from both sets | |
| func Union[T comparable](s1, s2 *Set[T]) *Set[T] { | |
| s3 := NewSet[T]() | |
| if s1 != nil && s1.items != nil { | |
| for k := range s1.items { | |
| s3.Add(k) | |
| } | |
| } | |
| if s2 != nil && s2.items != nil { | |
| for k := range s2.items { | |
| s3.Add(k) | |
| } | |
| } | |
| return s3 | |
| } |
🤖 Prompt for AI Agents
In challenge-27/submissions/Johrespi/solution-template.go around lines 228 to 243, the Union function ranges over s1.items and s2.items which will panic if s1 or s2 is a zero-value or has a nil items map; add nil-safe guards before iterating: check if s1 != nil && s1.items != nil before the first range and similarly for s2, so the function still returns a new empty set when either input is nil without causing a runtime panic. | // Difference returns a new set with elements in s1 that are not in s2 | ||
| func Difference[T comparable](s1, s2 *Set[T]) *Set[T] { | ||
| // TODO: Implement this function | ||
| | ||
| s3 := NewSet[T]() | ||
| | ||
| for k, _ := range s1.items { | ||
| if !s2.Contains(k) { | ||
| s3.Add(k) | ||
| } | ||
| } | ||
| return s3 | ||
| } |
There was a problem hiding this comment.
Handle nil set inputs.
The Difference function will panic if either s1 or s2 is a zero-value Set with nil maps. Add nil checks before ranging.
func Difference[T comparable](s1, s2 *Set[T]) *Set[T] { -// TODO: Implement this function - s3 := NewSet[T]() -for k, _ := range s1.items { -if !s2.Contains(k) { -s3.Add(k) +if s1 != nil && s1.items != nil && s2 != nil { +for k := range s1.items { +if !s2.Contains(k) { +s3.Add(k) +} } } + return s3 }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Difference returns a new set with elements in s1 that are not in s2 | |
| func Difference[T comparable](s1, s2 *Set[T]) *Set[T] { | |
| // TODO: Implement this function | |
| s3 := NewSet[T]() | |
| for k, _ := range s1.items { | |
| if !s2.Contains(k) { | |
| s3.Add(k) | |
| } | |
| } | |
| return s3 | |
| } | |
| // Difference returns a new set with elements in s1 that are not in s2 | |
| func Difference[T comparable](s1, s2 *Set[T]) *Set[T] { | |
| s3 := NewSet[T]() | |
| if s1 != nil && s1.items != nil && s2 != nil { | |
| for k := range s1.items { | |
| if !s2.Contains(k) { | |
| s3.Add(k) | |
| } | |
| } | |
| } | |
| return s3 | |
| } |
🤖 Prompt for AI Agents
In challenge-27/submissions/Johrespi/solution-template.go around lines 260 to 272, the Difference function can panic when s1 or s2 are zero-value sets with nil maps; add nil checks at the top: if s1 == nil || s1.items == nil return NewSet[T]() (nothing to iterate), and if s2 == nil || s2.items == nil set s2 = NewSet[T]() (treat as empty set) before the for-range so you never range over a nil map and behavior matches treating nil as an empty set. 
2 participants
Challenge 27 Solution
Submitted by: @Johrespi
Challenge: Challenge 27
Description
This PR contains my solution for Challenge 27.
Changes
challenge-27/submissions/Johrespi/solution-template.goTesting
Thank you for reviewing my submission! 🚀