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 NewStack[T 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] = NewStack[int]()
	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 NewStack[int]() 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.