sickn33/agentic-awesome-skills/go

// ❌ Ignoring errors
result, _ := os.Open(path)
 
// ✅ — always handle; only use _ when error is provably irrelevant
result, err := os.Open(path)
if err != nil {
    return fmt.Errorf("open %s: %w", path, err)
}

// ❌ Redundant error variable
err := doA()
if err != nil { return err }
err = doB()
if err != nil { return err }
 
// ✅ — each :=/:  is fine; this is idiomatic Go. Don't try to "fix" it.
// What you CAN simplify: collapsing to one-liners where the if body is a single return
if err := doA(); err != nil { return err }
if err := doB(); err != nil { return err }

// ❌ Custom error type with no added value
type MyError struct{ msg string }
func (e MyError) Error() string { return e.msg }
 
// ✅ — use errors.New or fmt.Errorf unless callers need to inspect type
var ErrNotFound = errors.New("not found")
return fmt.Errorf("lookup %q: %w", key, ErrNotFound)

// ❌ Growing a slice without pre-allocation when size is known
var result []string
for _, item := range items {
    result = append(result, item.Name)
}
 
// ✅
result := make([]string, 0, len(items))
for _, item := range items {
    result = append(result, item.Name)
}

// ❌ Manual existence check before map write
if _, ok := m[key]; !ok {
    m[key] = []string{}
}
m[key] = append(m[key], value)
 
// ✅ — append to nil slice is valid Go
m[key] = append(m[key], value)

// ❌ Copying a map by assignment (copies reference)
copy := original
 
// ✅
copy := make(map[K]V, len(original))
for k, v := range original { copy[k] = v }

// ❌ Fire-and-forget goroutine with no lifecycle
go doWork()
 
// ✅ — use errgroup or WaitGroup to track completion
var wg sync.WaitGroup
wg.Add(1)
go func() {
    defer wg.Done()
    doWork()
}()
wg.Wait()

// ❌ Unbuffered channel causing unnecessary goroutine block
ch := make(chan Result)
go func() { ch <- compute() }()
result := <-ch
 
// ✅ — for single-result, buffered channel avoids goroutine leak if receiver exits early
ch := make(chan Result, 1)
go func() { ch <- compute() }()
result := <-ch

// ❌ select with a busy-wait default
for {
    select {
    case v := <-ch:
        process(v)
    default:
        // spin
    }
}
 
// ✅ — blocking select unless you genuinely need non-blocking
for v := range ch {
    process(v)
}

// ❌ Large interface
type Storage interface {
    Get(key string) ([]byte, error)
    Set(key string, val []byte) error
    Delete(key string) error
    List(prefix string) ([]string, error)
    // ... 10 more methods
}
 
// ✅ — small, composable interfaces
type Getter interface { Get(key string) ([]byte, error) }
type Setter interface { Set(key string, val []byte) error }
type Storage interface { Getter; Setter }

// ❌ Returning concrete struct from constructor (ties callers to implementation)
func NewStore() *RedisStore { ... }
 
// ✅ — return interface when you have or anticipate multiple implementations
func NewStore() Storage { return &RedisStore{...} }

// ❌ Pointer receiver for tiny value types
func (p *Point) X() float64 { return p.x }
 
// ✅ — value receiver for small immutable types
func (p Point) X() float64 { return p.x }

// ❌ Named return values used just to avoid a variable declaration
func divide(a, b float64) (result float64, err error) {
    result = a / b
    return
}
 
// ✅ — named returns are worth it only for deferred mutation or documentation
func divide(a, b float64) (float64, error) {
    if b == 0 { return 0, errors.New("division by zero") }
    return a / b, nil
}

// ❌ Closure capturing loop variable (classic Go bug, fixed in Go 1.22+)
// Pre-1.22: each goroutine captures the same i
for i := 0; i < n; i++ {
    go func() { use(i) }()
}
 
// ✅ (Go <1.22 — pass as parameter)
for i := 0; i < n; i++ {
    go func(i int) { use(i) }(i)
}
// Go 1.22+: loop variable scoped per iteration, so the original is safe