Go Code Review

Go Code Review

Review Workflow

Follow this sequence in order. Do not emit findings until every Pass below is satisfied.

1. Baseline `go.mod` — Open go.mod and read the go directive. Pass: You can state the exact go X.YY value (in the review preamble or working notes). Apply version-gated advice only when it matches this baseline (loop capture pre-1.22, slog/structured logging from 1.21, errors.Join from 1.20).

2. Read surrounding code — For each changed .go file, read full functions or logical units that contain the edits, not only the diff hunk. Pass: At least one full enclosing function (or package-level init/var block) containing the change was read per changed file.

3. Scope the checklist — Decide which Review Checklist blocks apply (error handling, concurrency, interfaces/types, resources, naming). Load references for those blocks; skip blocks that are irrelevant to the diff. Pass: The review (or working notes) lists which checklist blocks you applied, or marks blocks N/A with a one-line reason tied to the diff (e.g. “no concurrency in change”).

4. Pre-report verification — Load and follow review-verification-protocol. Pass: The protocol’s Pre-Report Verification Checklist is satisfied for each finding you will report (actual code read, surrounding context checked, “wrong” vs “different style” distinguished, etc.).

Hard gates (same sequence, shorter)

Output Format

Report findings as:

[FILE:LINE] ISSUE_TITLE
Severity: Critical | Major | Minor | Informational
Description of the issue and why it matters.

Quick Reference

Review Checklist

Error Handling

• [ ] All errors checked (no _ = err without justifying comment)
• [ ] Errors wrapped with context (fmt.Errorf("...: %w", err))
• [ ] errors.Is/errors.As used instead of string matching
• [ ] errors.Join used for aggregating multiple errors (Go 1.20+)
• [ ] Zero values returned alongside errors

Concurrency

• [ ] No goroutine leaks (context cancellation or shutdown signal exists)
• [ ] Channels closed by sender only, exactly once
• [ ] Shared state protected by mutex or sync types
• [ ] WaitGroups used to wait for goroutine completion
• [ ] Context propagated through call chain
• [ ] Loop variable capture handled (pre-Go 1.22 codebases only)

Interfaces and Types

• [ ] Interfaces defined by consumers, not producers
• [ ] Interface names follow -er convention
• [ ] Interfaces minimal (1-3 methods)
• [ ] Concrete types returned from constructors
• [ ] any preferred over interface{} (Go 1.18+)
• [ ] Generics used where appropriate instead of any or code generation

Resources and Lifecycle

• [ ] Resources closed with defer immediately after creation
• [ ] HTTP response bodies always closed
• [ ] No defer in loops without closure wrapping
• [ ] init() functions avoided in favor of explicit initialization

Naming and Style

• [ ] Exported names have doc comments
• [ ] No stuttering names (user.UserService → user.Service)
• [ ] No naked returns in functions > 5 lines
• [ ] Context passed as first parameter
• [ ] slog used over log for structured logging (Go 1.21+)

Severity Calibration

Critical (Block Merge)

• Unchecked errors on I/O, network, or database operations
• Goroutine leaks (no shutdown path)
• Race conditions on shared state (concurrent map access without sync)
• Unbounded resource accumulation (defer in loop, unclosed connections)

Major (Should Fix)

• Errors returned without context (bare return err)
• Missing WaitGroup for spawned goroutines
• panic for recoverable errors
• Context not propagated to downstream calls

Minor (Consider Fixing)

• interface{} instead of any in Go 1.18+ codebases
• Missing doc comments on exports
• Stuttering names
• Slice not preallocated when size is known

Informational (Note Only)

• Suggestions to add generics where code generation exists
• Refactoring ideas for interface design
• Performance optimizations without measured impact

When to Load References

• Reviewing error return patterns → error-handling.md
• Reviewing goroutines, channels, or sync types → concurrency.md
• Reviewing type definitions, interfaces, or generics → interfaces.md
• General review (resources, naming, init, performance) → common-mistakes.md

Valid Patterns (Do NOT Flag)

These are acceptable Go patterns — reporting them wastes developer time:

• `_ = err` with reason comment — Intentionally ignored errors with explanation
• Empty interface / `any` — For truly generic code or interop with untyped APIs
• Naked returns in short functions — Acceptable in functions < 5 lines with named returns
• Channel without close — When consumer stops via context cancellation, not channel close
• Mutex protecting struct fields — Even if accessed only via methods, this is correct encapsulation
• `//nolint` directives with reason — Acceptable when accompanied by explanation
• Defer in loop — When function scope cleanup is intentional (e.g., processing files in batches)
• Functional options pattern — type Option func(*T) with With* constructors is idiomatic
• `sync.Pool` for hot paths — Acceptable for reducing allocation pressure in performance-critical code
• `context.Background()` in main/tests — Valid root context for top-level calls
• `select` with `default` — Non-blocking channel operation, intentional pattern
• Short variable names in small scope — i, err, ctx, ok are idiomatic Go

Context-Sensitive Rules

Only flag these issues when the specific conditions apply:

Before Submitting Findings

Satisfy step 4 in Review Workflow: load review-verification-protocol and complete its pre-report checks for each issue.

Common Mistakes

Resource Leaks

1. Missing defer for Close

Resources leaked on early return. The defer should come immediately after the error check for the open/create call.

// BAD
func readFile(path string) ([]byte, error) {
    f, err := os.Open(path)
    if err != nil {
        return nil, err
    }
    data, err := io.ReadAll(f)
    if err != nil {
        return nil, err  // file never closed!
    }
    f.Close()
    return data, nil
}

// GOOD - defer immediately
func readFile(path string) ([]byte, error) {
    f, err := os.Open(path)
    if err != nil {
        return nil, err
    }
    defer f.Close()
    return io.ReadAll(f)
}

2. Defer in Loop

defer runs at function exit, not loop iteration exit. In a loop, resources accumulate until the function returns.

// BAD - files stay open until function returns
for _, path := range paths {
    f, _ := os.Open(path)
    defer f.Close()
    process(f)
}

// GOOD - wrap in closure for per-iteration cleanup
for _, path := range paths {
    func() {
        f, _ := os.Open(path)
        defer f.Close()
        process(f)
    }()
}

3. HTTP Response Body Not Closed

Every http.Client call that returns a non-nil response has a body that must be closed, even if you don't read it. Failing to close it leaks the underlying TCP connection.

// BAD
resp, err := http.Get(url)
if err != nil {
    return err
}
data, _ := io.ReadAll(resp.Body)

// GOOD
resp, err := http.Get(url)
if err != nil {
    return err
}
defer resp.Body.Close()
data, _ := io.ReadAll(resp.Body)

Naming and Style

4. Stuttering Names

Package names are part of the identifier at the call site. Repeating the package name in the type or function name creates redundancy.

// BAD
package user
type UserService struct { ... }  // user.UserService

// GOOD
package user
type Service struct { ... }  // user.Service

5. Missing Doc Comments on Exports

Exported names without doc comments can't be documented by godoc/pkgsite. The comment should start with the name being documented.

// BAD
func NewServer(addr string) *Server { ... }

// GOOD
// NewServer creates a new HTTP server listening on addr.
func NewServer(addr string) *Server { ... }

6. Naked Returns in Long Functions

Named returns are convenient in short functions, but in longer functions they obscure what's being returned. The threshold is roughly 5 lines — beyond that, be explicit.

// BAD
func process(data []byte) (result string, err error) {
    // 50 lines of code...
    return  // what's being returned?
}

// GOOD - explicit returns
func process(data []byte) (string, error) {
    // 50 lines of code...
    return processedString, nil
}

Initialization

7. Init Function Overuse

init() functions run before main(), create hidden dependencies, make testing harder, and can cause subtle ordering issues when multiple packages have init functions.

// BAD - global state via init
var db *sql.DB

func init() {
    var err error
    db, err = sql.Open("postgres", os.Getenv("DATABASE_URL"))
    if err != nil {
        log.Fatal(err)
    }
}

// GOOD - explicit initialization
type App struct {
    db *sql.DB
}

func NewApp(dbURL string) (*App, error) {
    db, err := sql.Open("postgres", dbURL)
    if err != nil {
        return nil, fmt.Errorf("opening db: %w", err)
    }
    return &App{db: db}, nil
}

8. Global Mutable State

Package-level mutable variables create race conditions in concurrent code and make testing unreliable because tests share state.

// BAD
var config Config

func GetConfig() Config {
    return config
}

// GOOD - dependency injection
type Server struct {
    config Config
}

func NewServer(cfg Config) *Server {
    return &Server{config: cfg}
}

Structured Logging (Go 1.21+)

9. Using log Instead of slog

The log/slog package (Go 1.21+) provides structured, leveled logging that's far more useful in production than unstructured log.Println output.

// OLD - unstructured, hard to parse
log.Printf("failed to load user %d: %v", userID, err)

// MODERN - structured, machine-parseable
slog.Error("failed to load user",
    "user_id", userID,
    "error", err,
)

// With logger groups and attributes
logger := slog.With("service", "auth")
logger.Info("user logged in",
    "user_id", userID,
    "ip", req.RemoteAddr,
)

Key slog patterns:

• Use slog.With() to add common attributes to a logger
• Pass *slog.Logger as a dependency, don't use the global default in libraries
• Implement slog.LogValuer for custom types that appear frequently in logs
• Use slog.Group() to namespace related attributes

Performance

10. String Concatenation in Loop

String concatenation with + in a loop creates a new string allocation on every iteration, resulting in O(n^2) memory usage.

// BAD
var result string
for _, s := range items {
    result += s + ", "
}

// GOOD
var b strings.Builder
for _, s := range items {
    b.WriteString(s)
    b.WriteString(", ")
}
result := b.String()

11. Slice Preallocation

When you know the final size, preallocate to avoid repeated backing array copies as the slice grows.

// BAD - grows dynamically
var results []Result
for _, item := range items {
    results = append(results, process(item))
}

// GOOD - preallocate known size
results := make([]Result, 0, len(items))
for _, item := range items {
    results = append(results, process(item))
}

12. Range Over Integer (Go 1.22+)

Go 1.22 added range over integers, replacing the classic C-style for loop for simple counting:

// OLD
for i := 0; i < n; i++ {
    process(i)
}

// MODERN (Go 1.22+)
for i := range n {
    process(i)
}

Sync and Performance

13. sync.Pool Misuse

Objects returned to a sync.Pool must be reset first, otherwise the next consumer gets stale data.

// BAD - not resetting before Put
buf := bufPool.Get().(*bytes.Buffer)
buf.WriteString("data")
bufPool.Put(buf)  // still has "data"!

// GOOD - reset before returning to pool
buf := bufPool.Get().(*bytes.Buffer)
defer func() {
    buf.Reset()
    bufPool.Put(buf)
}()
buf.WriteString("data")

14. Functional Options

Constructors with many parameters are hard to read and painful to extend. The functional options pattern provides a clean API with sensible defaults.

// BAD - parameter bloat
func NewServer(addr string, timeout time.Duration, logger *slog.Logger, maxConns int) *Server

// GOOD - functional options
type Option func(*Server)

func WithTimeout(d time.Duration) Option {
    return func(s *Server) { s.timeout = d }
}

func NewServer(addr string, opts ...Option) *Server {
    s := &Server{addr: addr, timeout: 30 * time.Second}
    for _, opt := range opts {
        opt(s)
    }
    return s
}

Testing

15. Table-Driven Tests Missing

Table-driven tests reduce repetition and make it easy to add new cases.

// BAD
func TestAdd(t *testing.T) {
    if Add(1, 2) != 3 {
        t.Error("1+2 should be 3")
    }
    if Add(0, 0) != 0 {
        t.Error("0+0 should be 0")
    }
}

// GOOD
func TestAdd(t *testing.T) {
    tests := []struct {
        a, b, want int
    }{
        {1, 2, 3},
        {0, 0, 0},
        {-1, 1, 0},
    }
    for _, tt := range tests {
        got := Add(tt.a, tt.b)
        if got != tt.want {
            t.Errorf("Add(%d, %d) = %d, want %d", tt.a, tt.b, got, tt.want)
        }
    }
}

Review Questions

1. Is defer Close() called immediately after opening resources? 2. Are HTTP response bodies always closed? 3. Are package-level names not stuttering with package name? 4. Do exported symbols have doc comments? 5. Is mutable global state avoided? 6. Are slices preallocated when size is known? 7. Is slog used instead of log for structured output (Go 1.21+)?

Concurrency

Critical Anti-Patterns

1. Goroutine Leak

Goroutines that block forever consume memory and can accumulate over time, eventually exhausting resources.

// BAD - no way to stop the goroutine
func startWorker() {
    go func() {
        for {
            doWork()
        }
    }()
}

// GOOD - context cancellation
func startWorker(ctx context.Context) {
    go func() {
        for {
            select {
            case <-ctx.Done():
                return
            default:
                doWork()
            }
        }
    }()
}

2. Unbounded Channel Send

If the receiver dies or falls behind, the sender blocks forever. Always provide an escape hatch via context.

// BAD - blocks if nobody reads
ch <- result

// GOOD - respect context
select {
case ch <- result:
case <-ctx.Done():
    return ctx.Err()
}

3. Closing Channel Multiple Times

Closing a closed channel panics at runtime. The rule: only the sender closes the channel, and only once.

// BAD - potential double close
close(ch)
close(ch)  // panic!

// GOOD - only sender closes, once
func produce(ch chan<- int) {
    defer close(ch)
    for i := 0; i < 10; i++ {
        ch <- i
    }
}

4. Race Condition on Shared State

Concurrent reads and writes to maps, slices, or structs without synchronization cause data corruption and crashes.

// BAD - concurrent map access
var cache = make(map[string]int)
func Get(key string) int {
    return cache[key]  // race!
}
func Set(key string, val int) {
    cache[key] = val  // race!
}

// GOOD - mutex protection
var (
    cache   = make(map[string]int)
    cacheMu sync.RWMutex
)
func Get(key string) int {
    cacheMu.RLock()
    defer cacheMu.RUnlock()
    return cache[key]
}
func Set(key string, val int) {
    cacheMu.Lock()
    defer cacheMu.Unlock()
    cache[key] = val
}

// ALTERNATIVE - sync.Map for simple concurrent access patterns
var cache sync.Map
func Get(key string) (int, bool) {
    v, ok := cache.Load(key)
    if !ok {
        return 0, false
    }
    return v.(int), true
}

5. Missing WaitGroup

Without synchronization, the calling function may return before spawned goroutines finish their work.

// BAD - may exit before done
for _, item := range items {
    go process(item)
}
return  // goroutines may not finish

// GOOD
var wg sync.WaitGroup
for _, item := range items {
    wg.Add(1)
    go func(item Item) {
        defer wg.Done()
        process(item)
    }(item)
}
wg.Wait()

6. Loop Variable Capture (Pre-Go 1.22)

Go 1.22+ fixed this — each iteration gets its own variable. Only flag in codebases with go.mod specifying Go < 1.22.

// ISSUE in Go < 1.22 - all goroutines see the last item
for _, item := range items {
    go func() {
        process(item)  // captures loop variable
    }()
}

// FIX for Go < 1.22 - capture in closure parameter
for _, item := range items {
    go func(item Item) {
        process(item)
    }(item)
}

// Go 1.22+ - this is fine, each iteration has its own variable
for _, item := range items {
    go func() {
        process(item)  // safe
    }()
}

7. Context Not Propagated

When context isn't passed to downstream calls, cancellation signals don't reach them. This means timeouts and cancellation from the caller have no effect.

// BAD
func Handler(ctx context.Context) error {
    result := doWork()  // ignores ctx
    return nil
}

// GOOD
func Handler(ctx context.Context) error {
    result, err := doWork(ctx)
    if err != nil {
        return err
    }
    return nil
}

sync.OnceValue and sync.OnceFunc (Go 1.21+)

These replace the common sync.Once + package-level variable pattern with a cleaner API:

// OLD PATTERN
var (
    dbOnce sync.Once
    db     *sql.DB
)
func getDB() *sql.DB {
    dbOnce.Do(func() {
        db, _ = sql.Open("postgres", os.Getenv("DATABASE_URL"))
    })
    return db
}

// NEW PATTERN (Go 1.21+) - type-safe, no package variable
var getDB = sync.OnceValue(func() *sql.DB {
    db, _ := sql.Open("postgres", os.Getenv("DATABASE_URL"))
    return db
})

// With error handling
var getDB = sync.OnceValues(func() (*sql.DB, error) {
    return sql.Open("postgres", os.Getenv("DATABASE_URL"))
})

Worker Pool Pattern

func processItems(ctx context.Context, items []Item) error {
    const workers = 5

    jobs := make(chan Item)
    errs := make(chan error, 1)

    var wg sync.WaitGroup
    for i := 0; i < workers; i++ {
        wg.Add(1)
        go func() {
            defer wg.Done()
            for item := range jobs {
                if err := process(ctx, item); err != nil {
                    select {
                    case errs <- err:
                    default:
                    }
                    return
                }
            }
        }()
    }

    go func() {
        wg.Wait()
        close(errs)
    }()

    for _, item := range items {
        select {
        case jobs <- item:
        case err := <-errs:
            return err
        case <-ctx.Done():
            return ctx.Err()
        }
    }
    close(jobs)

    return <-errs
}

errgroup Pattern

The golang.org/x/sync/errgroup package simplifies the worker pool pattern with built-in context cancellation:

func processItems(ctx context.Context, items []Item) error {
    g, ctx := errgroup.WithContext(ctx)
    g.SetLimit(5)

    for _, item := range items {
        g.Go(func() error {
            return process(ctx, item)
        })
    }

    return g.Wait()
}

Review Questions

1. Are all goroutines stoppable via context? 2. Are channels always closed by the sender? 3. Is shared state protected by mutex or sync types? 4. Are WaitGroups used to wait for goroutine completion? 5. Is context passed through the call chain? 6. Is loop variable capture handled correctly for the target Go version? 7. Are sync.OnceValue/sync.OnceFunc used instead of sync.Once + variable (Go 1.21+)?

Error Handling

Critical Anti-Patterns

1. Ignoring Errors

Silent failures are impossible to debug.

// BAD
file, _ := os.Open("config.json")
data, _ := io.ReadAll(file)

// GOOD
file, err := os.Open("config.json")
if err != nil {
    return fmt.Errorf("opening config: %w", err)
}
defer file.Close()

2. Unwrapped Errors

Loses context for debugging. When an error bubbles up through multiple layers, each layer should add context about what it was trying to do.

// BAD - raw error
if err != nil {
    return err
}

// GOOD - wrapped with context
if err != nil {
    return fmt.Errorf("loading user %d: %w", userID, err)
}

3. String Errors Instead of Wrapping

Using %s or .Error() breaks the error chain — callers can no longer use errors.Is or errors.As to inspect the underlying cause.

// BAD - breaks error inspection
return fmt.Errorf("failed: %s", err.Error())
return fmt.Errorf("failed: %v", err)

// GOOD - preserves error chain
return fmt.Errorf("failed: %w", err)

4. Panic for Recoverable Errors

Panics crash the program and bypass normal error handling. Reserve them for truly unrecoverable situations (programmer bugs, violated invariants), not for expected failures like I/O errors.

// BAD
func GetConfig(path string) Config {
    data, err := os.ReadFile(path)
    if err != nil {
        panic(err)
    }
    ...
}

// GOOD
func GetConfig(path string) (Config, error) {
    data, err := os.ReadFile(path)
    if err != nil {
        return Config{}, fmt.Errorf("reading config: %w", err)
    }
    ...
}

5. Checking Error String Instead of Type

Error messages can change between releases. Type-based checking is stable.

// BAD
if err.Error() == "file not found" {
    ...
}

// GOOD
if errors.Is(err, os.ErrNotExist) {
    ...
}

// For custom errors
var ErrNotFound = errors.New("not found")
if errors.Is(err, ErrNotFound) {
    ...
}

6. Returning Error and Valid Value

Callers expect zero values when errors are returned. Returning a meaningful value alongside an error creates ambiguity about whether the value is usable.

// BAD - -1 is a valid integer, confuses callers
func Parse(s string) (int, error) {
    if s == "" {
        return -1, errors.New("empty string")
    }
    ...
}

// GOOD - zero value on error
func Parse(s string) (int, error) {
    if s == "" {
        return 0, errors.New("empty string")
    }
    ...
}

Multi-Error Aggregation (Go 1.20+)

When a function encounters multiple independent errors (cleanup, batch processing, parallel operations), combine them with errors.Join instead of dropping all but one.

// BAD - loses the first error
func cleanup(db *sql.DB, f *os.File) error {
    err := db.Close()
    err = f.Close()  // overwrites db error
    return err
}

// GOOD - preserves both errors
func cleanup(db *sql.DB, f *os.File) error {
    return errors.Join(db.Close(), f.Close())
}

errors.Join returns nil when all errors are nil, and the joined error supports errors.Is/errors.As for each constituent error:

err := errors.Join(ErrNotFound, ErrTimeout)
errors.Is(err, ErrNotFound) // true
errors.Is(err, ErrTimeout)  // true

This is especially useful in defer chains:

func processFile(path string) (retErr error) {
    f, err := os.Open(path)
    if err != nil {
        return fmt.Errorf("opening %s: %w", path, err)
    }
    defer func() {
        retErr = errors.Join(retErr, f.Close())
    }()
    // ... process file
}

Sentinel Errors Pattern

// Define at package level
var (
    ErrNotFound     = errors.New("not found")
    ErrUnauthorized = errors.New("unauthorized")
)

// Usage
func GetUser(id int) (*User, error) {
    user := db.Find(id)
    if user == nil {
        return nil, ErrNotFound
    }
    return user, nil
}

// Caller checks
if errors.Is(err, ErrNotFound) {
    http.Error(w, "User not found", 404)
}

Custom Error Types

When you need to carry structured data with an error, implement the error interface:

type ValidationError struct {
    Field   string
    Message string
}

func (e *ValidationError) Error() string {
    return fmt.Sprintf("validation failed on %s: %s", e.Field, e.Message)
}

// Caller extracts structured data
var ve *ValidationError
if errors.As(err, &ve) {
    log.Printf("field %s: %s", ve.Field, ve.Message)
}

Review Questions

1. Are all error returns checked (no _)? 2. Are errors wrapped with context using %w? 3. Are sentinel errors used for expected error conditions? 4. Does the code use errors.Is/As instead of string matching? 5. Does it return zero values alongside errors? 6. Are multiple independent errors aggregated with errors.Join?

Interfaces and Types

Critical Anti-Patterns

1. Premature Interface Definition

Interfaces should be defined where they're consumed, not where the implementation lives. Defining them in the producer package couples the abstraction to a specific implementation.

// BAD - interface in producer package
package storage

type UserRepository interface {
    Get(id int) (*User, error)
    Save(user *User) error
}

type PostgresUserRepository struct { ... }

// GOOD - interface in consumer package
package service

type UserGetter interface {
    Get(id int) (*User, error)
}

func NewUserService(users UserGetter) *UserService {
    return &UserService{users: users}
}

2. Interface Pollution (Too Many Methods)

Fat interfaces are hard to implement, hard to mock, and force consumers to depend on methods they don't use.

// BAD - fat interface
type UserStore interface {
    Get(id int) (*User, error)
    GetAll() ([]*User, error)
    Save(user *User) error
    Delete(id int) error
    Search(query string) ([]*User, error)
    Count() (int, error)
}

// GOOD - focused interfaces composed as needed
type UserGetter interface {
    Get(id int) (*User, error)
}

type UserSaver interface {
    Save(user *User) error
}

type UserStore interface {
    UserGetter
    UserSaver
}

3. Wrong Interface Names

Go convention: single-method interfaces are named after the method with an -er suffix.

// BAD
type IUserService interface { ... }  // Java-style prefix
type UserServiceInterface { ... }    // redundant suffix
type UserManager interface { ... }   // vague noun

// GOOD - verb forms ending in -er
type UserReader interface {
    ReadUser(id int) (*User, error)
}

type UserWriter interface {
    WriteUser(user *User) error
}

4. Returning Interface Instead of Concrete Type

Returning interfaces from constructors hides information from callers and prevents them from accessing implementation-specific methods. Accept interfaces, return structs.

// BAD - returns interface
func NewServer(addr string) Server {
    return &httpServer{addr: addr}
}

// GOOD - returns concrete type
func NewServer(addr string) *HTTPServer {
    return &HTTPServer{addr: addr}
}

5. Interface for Single Implementation

An interface with only one implementation adds indirection without benefit. Introduce interfaces when you actually need them (testing, multiple implementations, package boundary decoupling).

// BAD - interface with only one implementation and no tests mocking it
type ConfigLoader interface {
    Load() (*Config, error)
}

type fileConfigLoader struct { ... }

// GOOD - just use the concrete type until you need the abstraction
type ConfigLoader struct { ... }

func (c *ConfigLoader) Load() (*Config, error) { ... }

Generics (Go 1.18+)

Prefer any over interface{}

The any keyword is an alias for interface{} introduced in Go 1.18. It's clearer and more idiomatic in modern Go code.

// OLD
func Process(data interface{}) interface{} { ... }

// MODERN
func Process(data any) any { ... }

Use Type Constraints Instead of any

When you know the set of types you need, use constraints to preserve type safety. any in a generic function means you've given up type checking.

// BAD - any constraint means no useful operations
func Max[T any](a, b T) T {
    // Can't compare a and b!
}

// GOOD - constrained to comparable and ordered types
func Max[T cmp.Ordered](a, b T) T {
    if a > b {
        return a
    }
    return b
}

Common Generic Anti-Patterns

// BAD - generic function that only works with one type
func ParseUserID[T ~string](s T) (int, error) {
    return strconv.Atoi(string(s))
}
// Just use string directly

// BAD - over-genericized struct
type Cache[K comparable, V any] struct { ... }
// Only used as Cache[string, *User] throughout the codebase
// Generics add value when there are multiple instantiations

// GOOD - generics for truly reusable code
func Map[T, U any](slice []T, fn func(T) U) []U {
    result := make([]U, len(slice))
    for i, v := range slice {
        result[i] = fn(v)
    }
    return result
}

Type Constraints with ~ (Underlying Types)

The ~ prefix matches types with the same underlying type, which is important for custom types:

type UserID int64

// Without ~: only accepts int64, not UserID
func Format[T int64](id T) string { ... }

// With ~: accepts int64 AND UserID
func Format[T ~int64](id T) string { ... }

Accept Interfaces, Return Structs

// Function accepts interface (flexible)
func WriteData(w io.Writer, data []byte) error {
    _, err := w.Write(data)
    return err
}

// Function returns concrete type (explicit)
func NewBuffer() *bytes.Buffer {
    return &bytes.Buffer{}
}

// Usage
buf := NewBuffer()
WriteData(buf, []byte("hello"))  // Buffer implements io.Writer

Standard Library Interfaces to Use

Prefer these over custom interfaces when your use case matches:

Review Questions

1. Are interfaces defined where they're used (consumer side)? 2. Are interfaces minimal (1-3 methods)? 3. Do interface names end in -er? 4. Are concrete types returned from constructors? 5. Is any used instead of interface{} (Go 1.18+)? 6. Are generics used where they add real value (multiple instantiations)? 7. Are type constraints specific enough (not just any)?