docs: add when-not to interfaces + error-handling + concurrency

2026-04-30 13:26:20 +00:00
parent a7a853bb43
commit 11048ae73e
3 changed files with 380 additions and 0 deletions
@@ -71,6 +71,52 @@ func (s *Stats) RecordHit() {
 }
 ```
 ### When NOT to Use
 **Don't use this when:**
 - Goroutines need to coordinate or communicate (not just protect state) — use channels
 - The critical section involves blocking I/O (holding a mutex during network calls starves other goroutines)
 - You can restructure to have a single goroutine own the state (no sharing = no lock needed)
 **Over-application example:**
 ```go
 // Using a mutex to coordinate work between goroutines
 type TaskQueue struct {
    mu    sync.Mutex
    tasks []Task
    ready bool
 }
 func (q *TaskQueue) WaitForReady() {
    for {
        q.mu.Lock()
        if q.ready {
            q.mu.Unlock()
            return
        }
        q.mu.Unlock()
        time.Sleep(10 * time.Millisecond) // spin-waiting with a lock — terrible
    }
 }
 ```
 **Better alternative:**
 ```go
 // Use a channel for coordination/signaling
 type TaskQueue struct {
    tasks chan Task
    ready chan struct{}
 }
 func (q *TaskQueue) WaitForReady() {
    <-q.ready // blocks efficiently, no spinning
 }
 ```
 **Why:** Mutexes protect data; channels coordinate goroutines. If you're polling a mutex-protected flag, you've reinvented a bad channel. The Go proverb applies: "Don't communicate by sharing memory; share memory by communicating."
 ### Idiomatic Usage
 ```go
@@ -168,6 +214,63 @@ func GetDB() *sql.DB {
 }
 ```
 ### When NOT to Use
 **Don't use this when:**
 - Initialization can fail and you need to retry — Once runs the func at most once, even on failure
 - You need to reset/reinitialize later (Once has no Reset method)
 - The initialization is cheap — just do it in `init()` or at declaration time
 **Over-application example:**
 ```go
 var (
    conn *grpc.ClientConn
    once sync.Once
 )
 func GetConn() (*grpc.ClientConn, error) {
    var err error
    once.Do(func() {
        conn, err = grpc.Dial("server:443")
    })
    if err != nil {
        return nil, err // PROBLEM: next call to GetConn returns nil conn, nil err
        // because once.Do won't run again
    }
    return conn, nil
 }
 ```
 **Better alternative:**
 ```go
 // Use sync.OnceValues (Go 1.21+) which caches both value and error,
 // or handle retry logic explicitly
 var getConn = sync.OnceValues(func() (*grpc.ClientConn, error) {
    return grpc.Dial("server:443")
 })
 // Or for retry scenarios, use a mutex with a nil check
 var (
    mu   sync.Mutex
    conn *grpc.ClientConn
 )
 func GetConn() (*grpc.ClientConn, error) {
    mu.Lock()
    defer mu.Unlock()
    if conn != nil {
        return conn, nil
    }
    var err error
    conn, err = grpc.Dial("server:443") // retries on next call if this fails
    return conn, err
 }
 ```
 **Why:** `sync.Once` guarantees exactly-once execution regardless of success or failure. If the initialization can fail transiently (network timeout, service unavailable), Once will permanently cache the failure. Use `sync.OnceValues` for caching results, or a mutex with a nil-check pattern when retry is needed.
 ### Idiomatic Usage
 ```go
@@ -409,6 +512,60 @@ func (s *Server) worker() {
 func (s *Server) Stop() { close(s.done) } // broadcasts to ALL workers
 ```
 ### When NOT to Use
 **Don't use this when:**
 - You need to send actual data between goroutines — use a typed channel
 - You only have one goroutine to signal — a simple `return` or function call suffices
 - You're using it as a poor substitute for `context.Context` (which already provides Done())
 **Over-application example:**
 ```go
 // Rolling your own done channel when context already provides this
 type Worker struct {
    done   chan struct{}
    ctx    context.Context
    cancel context.CancelFunc
 }
 func (w *Worker) Start() {
    go func() {
        select {
        case <-w.done:   // redundant — ctx.Done() does the same thing
            return
        case <-w.ctx.Done():
            return
        }
    }()
 }
 ```
 **Better alternative:**
 ```go
 // Just use the context — it already IS a done signal
 type Worker struct {
    ctx    context.Context
    cancel context.CancelFunc
 }
 func (w *Worker) Start() {
    go func() {
        select {
        case <-w.ctx.Done():
            return
        case work := <-w.workCh:
            process(work)
        }
    }()
 }
 func (w *Worker) Stop() { w.cancel() }
 ```
 **Why:** If you already have a `context.Context`, its `Done()` channel is your cancellation signal. Adding a separate `chan struct{}` duplicates functionality and creates two shutdown paths that must be kept in sync. Use raw done channels only when you don't have a context (e.g., standalone libraries that predate context).
 ### Anti-pattern
 ```go
@@ -648,6 +805,49 @@ func processAll(ctx context.Context, items []string) []Result {
 }
 ```
 ### When NOT to Use
 **Don't use this when:**
 - The "pipeline" has only one stage — you're just adding goroutine overhead to sequential code
 - All items are already in memory and processing is CPU-bound with no I/O — `for` loop is simpler and faster
 - You don't actually need backpressure (data fits in memory, producer is finite)
 **Over-application example:**
 ```go
 // Channel pipeline for a simple in-memory transformation
 func doubleAll(nums []int) []int {
    ch := make(chan int)
    go func() {
        defer close(ch)
        for _, n := range nums {
            ch <- n // channel overhead for no benefit
        }
    }()
    var result []int
    for n := range ch {
        result = append(result, n*2)
    }
    return result
 }
 ```
 **Better alternative:**
 ```go
 // Just use a loop — no concurrency needed
 func doubleAll(nums []int) []int {
    result := make([]int, len(nums))
    for i, n := range nums {
        result[i] = n * 2
    }
    return result
 }
 ```
 **Why:** Channel pipelines shine when stages involve I/O (network, disk) and can overlap waiting. For pure computation on in-memory data, the channel send/receive overhead (~50-100ns per item) adds up and the goroutine scheduling has no useful work to overlap with. A plain loop is faster, simpler, and easier to debug.
 ### Anti-pattern
 ```go
@@ -197,6 +197,37 @@ func ExampleProcess() {
 // ← go test verifies this compiles and produces the expected output
 ```
 **When NOT to Use**
 **Don't write Example tests when:**
 - The function signature is self-explanatory (`func Max(a, b int) int` doesn't need an example)
 - The example would just restate the doc comment with no additional insight
 - You're testing internal/unexported functions (examples must use the public API)
 - The output is non-deterministic (timestamps, random values, goroutine ordering)
 **Over-application example:**
 ```go
 // Pointless — the signature tells you everything
 func ExampleAbs() {
    fmt.Println(math.Abs(-5))
    // Output:
    // 5
 }
 ```
 **Better alternative:**
 ```go
 // Skip the example for trivial functions. Write examples for non-obvious behavior:
 func ExampleAbs_nan() {
    fmt.Println(math.Abs(math.NaN()))
    // Output:
    // NaN
 }
 // ← This teaches something surprising that the signature doesn't convey
 ```
 **Why:** Examples exist to teach usage patterns that aren't obvious from the type signature and doc comment. Trivial examples add maintenance burden without teaching anything.
 **Anti-pattern:** Examples that don't compile; examples without Output comments
 (not verified); examples in README that drift from reality.
@@ -319,6 +350,36 @@ func ParseDuration(s string) (time.Duration, error) {
 }
 ```
 **When NOT to Use**
 **Don't deprecate when:**
 - The function still works fine and there's no better replacement yet
 - You're deprecating to "clean up" the API without a migration path (users will just ignore it)
 - The replacement has a different behavior or isn't a drop-in substitute (document the difference instead)
 - You're on v0.x and can just remove it (pre-1.0 allows breaking changes)
 **Over-application example:**
 ```go
 // Deprecated: Use NewClientV2 instead.
 func NewClient(addr string) *Client { ... }
 // But NewClientV2 has a completely different API:
 func NewClientV2(cfg Config) (*ClientV2, error) { ... }
 // Users can't just find-and-replace — this isn't a deprecation, it's a migration
 ```
 **Better alternative:**
 ```go
 // NewClient creates a client with default configuration.
 // For advanced configuration (TLS, timeouts, connection pooling),
 // use [NewClientWithConfig] instead.
 func NewClient(addr string) *Client {
    return NewClientWithConfig(Config{Addr: addr})
 }
 ```
 **Why:** Deprecation means "there's a better way to do the same thing." If the replacement requires a fundamentally different approach, provide a migration guide — don't just slap `Deprecated:` on it and leave users stranded.
 **Anti-pattern:** Removing deprecated APIs (breaks semver); deprecating without
 suggesting an alternative; using non-standard deprecation markers.
@@ -74,6 +74,48 @@ func fetchUser(id int) (*User, error) {
 }
 ```
 ### When NOT to Use
 **Don't use this when:**
 - The error condition is internal and no caller should branch on it — use `fmt.Errorf` instead
 - You have too many sentinels (>5-6 per package) — consider a custom error type with a code field
 - The error carries varying context (file path, user ID) — sentinels are for fixed conditions
 **Over-application example:**
 ```go
 // A sentinel for every possible failure — explosion of package-level vars
 var (
    ErrUserNotFound     = errors.New("users: not found")
    ErrUserInactive     = errors.New("users: inactive")
    ErrUserSuspended    = errors.New("users: suspended")
    ErrUserRateLimited  = errors.New("users: rate limited")
    ErrUserInvalidEmail = errors.New("users: invalid email")
    ErrUserInvalidName  = errors.New("users: invalid name")
    ErrUserInvalidAge   = errors.New("users: invalid age")
    // 20 more...
 )
 ```
 **Better alternative:**
 ```go
 // Use a typed error with a code when you have many distinct conditions
 type UserError struct {
    Code    string // "not_found", "inactive", "suspended"
    Field   string // which field failed validation
    Message string
 }
 func (e *UserError) Error() string { return "users: " + e.Message }
 // Callers use errors.As to inspect
 var uerr *UserError
 if errors.As(err, &uerr) && uerr.Code == "not_found" { ... }
 ```
 **Why:** Sentinels are for a small number of well-known states that callers frequently branch on. If you're creating dozens, you've outgrown the pattern — a structured error type with an enum/code field scales better and avoids polluting the package namespace.
 ### Anti-pattern
 ```go
@@ -217,6 +259,43 @@ func loadConfig(path string) (*Config, error) {
 - **%w**: When the wrapped error is part of your API contract. Callers can depend on it.
 - **%v**: When you want to include the error text but NOT let callers depend on the underlying type. Use for implementation details.
 ### When NOT to Use
 **Don't use this when:**
 - You're wrapping at every single layer — the error message becomes `"a: b: c: d: e: permission denied"`
 - The added context is obvious from the function name (the caller already knows what function they called)
 - You're wrapping errors from a dependency you don't want in your API contract (use `%v` instead)
 **Over-application example:**
 ```go
 func (s *Server) handleRequest(w http.ResponseWriter, r *http.Request) {
    user, err := s.getUser(r)
    if err != nil {
        // "handleRequest: getUser: fetchFromDB: queryRow: scan: sql: no rows"
        // The handler name adds nothing — the caller IS the handler
        http.Error(w, fmt.Errorf("handleRequest: %w", err).Error(), 500)
        return
    }
 }
 ```
 **Better alternative:**
 ```go
 func (s *Server) handleRequest(w http.ResponseWriter, r *http.Request) {
    user, err := s.getUser(r)
    if err != nil {
        // Log the full chain internally, return a clean message to the client
        slog.Error("fetching user", "err", err, "path", r.URL.Path)
        http.Error(w, "internal error", 500)
        return
    }
 }
 ```
 **Why:** Wrapping should add *meaningful* context that isn't already obvious. At HTTP handler boundaries, you typically log the error (with full chain) and return a generic response. Wrapping at every layer creates unreadable error strings and leaks internals.
 ### Anti-pattern
 ```go
@@ -419,6 +498,46 @@ func cleanup(db *sql.DB, cache *redis.Client, file *os.File) error {
 }
 ```
 ### When NOT to Use
 **Don't use this when:**
 - Errors are causally related (error B was caused by error A) — use wrapping (`%w`) instead
 - You're collecting errors across retries of the *same* operation — return the last error or wrap them causally
 - The caller needs to distinguish which resource failed — Join loses that information
 **Over-application example:**
 ```go
 func fetchWithRetry(url string, attempts int) error {
    var errs []error
    for i := 0; i < attempts; i++ {
        err := fetch(url)
        if err == nil {
            return nil
        }
        errs = append(errs, err) // collecting retry errors — misleading
    }
    return errors.Join(errs...) // caller sees 3 errors but they're the SAME operation
 }
 ```
 **Better alternative:**
 ```go
 func fetchWithRetry(url string, attempts int) error {
    var lastErr error
    for i := 0; i < attempts; i++ {
        lastErr = fetch(url)
        if lastErr == nil {
            return nil
        }
    }
    return fmt.Errorf("fetch %s: %d attempts failed, last: %w", url, attempts, lastErr)
 }
 ```
 **Why:** `errors.Join` is for *independent* failures that all matter equally. For retries, the caller cares about the final failure and the retry count, not a list of (often identical) errors. For causal chains, wrapping preserves the relationship between cause and effect.
 ### Anti-pattern
 ```go