A Stack Data Structure

Asked to implement a stack in Go. Here's a generic, thread-safe, interface-driven implementation with proper error handling. package main import ( "errors" "fmt" "sync" ) // ErrStackEmpty is returned when attempting to pop or peek an empty stack. var ErrStackEmpty = errors.New("stack: operation on empty stack") // Stacker defines the contract for any stack-like data structure. type Stacker[T any] interface { Push(item T) Pop() (T, error) Peek() (T, error) Size() int IsEmpty() bool } // Stack is a generic, thread-safe LIFO data structure. type Stack[T any] struct { mu sync.RWMutex items []T } // NewStack constructs a new empty Stack. func NewStackT any *Stack[T] { return &Stack[T]{ items: make([]T, 0), } } // Push adds an item to the top of the stack. func (s *Stack[T]) Push(item T) { s.mu.Lock() defer s.mu.Unlock() s.items = append(s.items, item) } // Pop removes and returns the top item of the stack. func (s *Stack[T]) Pop() (T, error) { s.mu.Lock() defer s.mu.Unlock() var zero T if len(s.items) == 0 { return zero, ErrStackEmpty } n := len(s.items) - 1 item := s.items[n] s.items = s.items[:n] return item, nil } // Peek returns the top item without removing it. func (s *Stack[T]) Peek() (T, error) { s.mu.RLock() defer s.mu.RUnlock() var zero T if len(s.items) == 0 { return zero, ErrStackEmpty } return s.items[len(s.items)-1], nil } // Size returns the number of items in the stack. func (s *Stack[T]) Size() int { s.mu.RLock() defer s.mu.RUnlock() return len(s.items) } // IsEmpty reports whether the stack contains no items. func (s *Stack[T]) IsEmpty() bool { return s.Size() == 0 } func main() { var s Stacker[int] = NewStackint s.Push(1) s.Push(2) s.Push(3) fmt.Println("Size:", s.Size()) if top, err := s.Peek(); err == nil { fmt.Println("Top:", top) } for !s.IsEmpty() { val, _ := s.Pop() fmt.Println("Popped:", val) } } Code Review 1. Lines 13-19. Do we actually need a Stacker interface here? There's exactly one implementation and nothing accepts it as a parameter. Classic 'just in case' interface. 2. Lines 22-24. sync.RWMutex on a stack. Was concurrent access a requirement? Because nobody mentioned it in the ticket, and now every Push pays for a lock it doesn't need. 3. Lines 28-31. make([]T, 0) is equivalent to leaving it nil for append purposes. Not wrong, just ceremonial. 4. Lines 73-76. IsEmpty calls Size(), which acquires the RLock, then IsEmpty returns. Fine, but if a caller does IsEmpty() then Pop() they still race. The locking gives a false sense of safety. 5. Line 78. Assigning NewStackint to a Stacker[int] variable in main just to prove the interface exists. We see what you did there. 6. Lines 9-10. The error message 'stack: operation on empty stack' is fine, but we only ever return it from two places that already have obvious context. A sentinel error works, just noting the prefix is doing nothing for us. 7. Lines 33-37. Every method has a comment that restates the method name. 'Push adds an item to the top of the stack.' Thank you, I was worried Push might subtract.