Add solution for Challenge 27 by hvijaycse#1451
Conversation
WalkthroughAdds a single Go file implementing a generics library: Pair, Stack, Queue, Set (mutex-protected), ErrEmptyCollection, and slice utilities (Filter, Map, Reduce, Contains, FindIndex, RemoveDuplicates), plus set algebra functions (Union, Intersection, Difference). Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 2
🧹 Nitpick comments (7)
challenge-27/submissions/hvijaycse/solution-template.go (7)
52-63: Remove leftover TODO comment.The
Popmethod is fully implemented, but the TODO comment on line 53 remains. This should be cleaned up.🧹 Proposed fix
// Pop removes and returns the top element from the stack // Returns an error if the stack is empty func (s *Stack[T]) Pop() (T, error) { - // TODO: Implement this method var zero T if s.IsEmpty() {
113-122: Potential memory leak in slice-based queue.Reslicing with
q.Items = q.Items[1:]doesn't release the reference to the dequeued element in the underlying array. For queues holding pointers or large structs, this can cause memory to accumulate.♻️ Proposed fix to clear the reference
func (q *Queue[T]) Dequeue() (T, error) { var zero T if q.IsEmpty() { return zero, ErrEmptyCollection } - zero = q.Items[0] - q.Items = q.Items[1:] - return zero, nil + result := q.Items[0] + var empty T + q.Items[0] = empty // Clear reference to allow GC + q.Items = q.Items[1:] + return result, nil }
141-145: Remove leftover TODO comment.The
IsEmptymethod is implemented, but the TODO comment remains.🧹 Proposed fix
// IsEmpty returns true if the queue contains no elements func (q *Queue[T]) IsEmpty() bool { - // TODO: Implement this method return q.Size() == 0 }
212-250: Consider idiomatic Go naming for local variables.Go convention uses
camelCasefor local variables, notsnake_case. Names likenw_set,intr_set, anddiff_setcould benewSet,intersectSet(or justresult), anddiffSet.♻️ Proposed fix
func Union[T comparable](s1, s2 *Set[T]) *Set[T] { - - nw_set := NewSet[T]() + result := NewSet[T]() for _, val := range s1.Elements() { - nw_set.Add(val) + result.Add(val) } for _, val := range s2.Elements() { - nw_set.Add(val) + result.Add(val) } - return nw_set + return result } // Intersection returns a new set containing only elements that exist in both sets func Intersection[T comparable](s1, s2 *Set[T]) *Set[T] { - intr_set := NewSet[T]() + result := NewSet[T]() for _, val := range s1.Elements() { if !s2.Contains(val) { continue } - - intr_set.Add(val) + result.Add(val) } - return intr_set + return result } // Difference returns a new set with elements in s1 that are not in s2 func Difference[T comparable](s1, s2 *Set[T]) *Set[T] { - diff_set := NewSet[T]() + result := NewSet[T]() for _, val := range s1.Elements() { if s2.Contains(val) { continue } - - diff_set.Add(val) + result.Add(val) } - return diff_set + return result }
157-163: Remove leftover TODO comment.The
NewSetfunction is implemented, but the TODO comment remains.🧹 Proposed fix
// NewSet creates a new empty set func NewSet[T comparable]() *Set[T] { - // TODO: Implement this function return &Set[T]{ ItemSet: map[T]struct{}{}, } }
165-171: Remove leftover TODO comment.🧹 Proposed fix
// Add adds an element to the set if it's not already present func (s *Set[T]) Add(value T) { - // TODO: Implement this method s.mu.Lock() defer s.mu.Unlock() s.ItemSet[value] = struct{}{} }
256-325: LGTM!Utility functions are correctly implemented. Minor nit:
mapped_vals(line 271) uses snake_case; Go convention prefersmappedVals.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
challenge-27/submissions/hvijaycse/solution-template.go (2)
52-62: Consider clearer variable naming inPopandPeek.The variable
zerois reused to hold the actual return value, which is confusing. Consider using distinct names for the zero value and the result.♻️ Suggested improvement
func (s *Stack[T]) Pop() (T, error) { var zero T if s.IsEmpty() { return zero, ErrEmptyCollection } length := len(s.Items) - zero = s.Items[length-1] + value := s.Items[length-1] s.Items = s.Items[:length-1] - return zero, nil + return value, nil }
206-243: Consider Go naming conventions for local variables.Go idiom uses camelCase for local variables. Variables like
nw_set,intr_set, anddiff_setwould be more idiomatic asnewSet,intrSet/result, anddiffSet/result.♻️ Example for Union
func Union[T comparable](s1, s2 *Set[T]) *Set[T] { - nw_set := NewSet[T]() + result := NewSet[T]() for _, val := range s1.Elements() { - nw_set.Add(val) + result.Add(val) } for _, val := range s2.Elements() { - nw_set.Add(val) + result.Add(val) } - return nw_set + return result }
2 participants
Challenge 27 Solution
Submitted by: @hvijaycse
Challenge: Challenge 27
Description
This PR contains my solution for Challenge 27.
Changes
challenge-27/submissions/hvijaycse/solution-template.goTesting
Thank you for reviewing my submission! 🚀