docs: add 'when to use' triggers + examples to all patterns
Added 'When to Use' subsections with concrete decision triggers and before/after Go code examples to patterns across all directories: - patterns/error-handling.md (3 patterns: sentinels, wrapping, Join) - patterns/concurrency.md (4 patterns: Mutex, Once, done channels, pipelines) - patterns/interfaces.md (4 patterns: small interfaces, accept/return, adapter, optional) - patterns/structs.md (3 patterns: zero-value, constructors, config structs) - patterns/package-design.md (3 patterns: internal/, init(), context keys) - patterns/style.md (3 patterns: interface checks, iota constants, named types) - patterns/testing-advanced.md (3 patterns: table tests, golden files, httptest) - patterns/api-conventions.md (3 patterns: Must, layered API, graceful shutdown) - patterns/documentation.md (2 patterns: examples, deprecated) - kubernetes/patterns.md (3 patterns: controller, workqueue, leader election) - kubernetes/production-go.md (2 patterns: codegen, HandleCrash) - smells/anti-patterns.md (2 anti-patterns: cache mutation, edge-triggered)
This commit is contained in:
@@ -13,6 +13,32 @@ is non-nil. Named `MustXxx` or `Must` (when wrapping a generic `(T, error)` pair
|
||||
`var` initializers can't handle errors, `Must` converts programmer errors (bad
|
||||
regex literals, bad templates) into immediate panics that surface during init.
|
||||
|
||||
**When to Use**
|
||||
|
||||
**Triggers:**
|
||||
- You're initializing a package-level variable with a value that is known at compile time (regex, template, URL)
|
||||
- Failure means a programmer bug, not a runtime condition (the regex literal is wrong, not user input)
|
||||
- `var` initialization can't handle the `(T, error)` return
|
||||
|
||||
**Example — before:**
|
||||
```go
|
||||
var emailRegex *regexp.Regexp
|
||||
|
||||
func init() {
|
||||
var err error
|
||||
emailRegex, err = regexp.Compile(`^[a-z]+@[a-z]+\.[a-z]+$`)
|
||||
if err != nil {
|
||||
panic(err) // manual panic, verbose
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Example — after:**
|
||||
```go
|
||||
var emailRegex = regexp.MustCompile(`^[a-z]+@[a-z]+\.[a-z]+$`)
|
||||
// One line. Panics on typo (caught immediately in tests). Clean.
|
||||
```
|
||||
|
||||
**Anti-pattern:** Using Must in runtime code where the input is dynamic/user-provided;
|
||||
panicking on recoverable errors; naming it something other than Must (e.g., `PanicOnError`).
|
||||
|
||||
@@ -211,6 +237,29 @@ func (b *Builder) String() string {
|
||||
**Why:** 90% of file opens are reads or creates. Layered APIs serve the common case
|
||||
without hiding power. The naming makes intent clear.
|
||||
|
||||
**When to Use**
|
||||
|
||||
**Triggers:**
|
||||
- 90% of callers need the simple case (open for read, create and truncate)
|
||||
- You have a powerful function with many flags/options but most combinations are rare
|
||||
- You find yourself writing the same flag combination repeatedly in calling code
|
||||
|
||||
**Example — before:**
|
||||
```go
|
||||
// User must know about flags for every file open
|
||||
f, err := os.OpenFile("data.json", os.O_RDONLY, 0)
|
||||
// Every. Single. Time.
|
||||
```
|
||||
|
||||
**Example — after:**
|
||||
```go
|
||||
// Simple case:
|
||||
f, err := os.Open("data.json") // just reads — no flags to remember
|
||||
|
||||
// Power case (when you actually need it):
|
||||
f, err := os.OpenFile("data.json", os.O_RDWR|os.O_CREATE|os.O_APPEND, 0644)
|
||||
```
|
||||
|
||||
**Anti-pattern:** Only exposing the full-power version; making users learn flag
|
||||
constants for simple reads; duplicating implementation across convenience functions.
|
||||
|
||||
@@ -311,6 +360,48 @@ timeout mechanism.
|
||||
Graceful shutdown is critical for production services; immediate close is needed for
|
||||
tests and emergency stops.
|
||||
|
||||
**When to Use**
|
||||
|
||||
**Triggers:**
|
||||
- Your type manages long-lived connections or in-flight requests
|
||||
- You need both "stop now" (tests, emergencies) and "drain gracefully" (deploys, SIGTERM)
|
||||
- A `Close()` that waits forever would make tests hang
|
||||
|
||||
**Example — before:**
|
||||
```go
|
||||
type Server struct { listener net.Listener }
|
||||
|
||||
func (s *Server) Stop() {
|
||||
s.listener.Close() // all in-flight requests get connection reset — data loss
|
||||
}
|
||||
```
|
||||
|
||||
**Example — after:**
|
||||
```go
|
||||
type Server struct {
|
||||
listener net.Listener
|
||||
active sync.WaitGroup
|
||||
}
|
||||
|
||||
// Immediate: drop everything
|
||||
func (s *Server) Close() error {
|
||||
return s.listener.Close()
|
||||
}
|
||||
|
||||
// Graceful: stop accepting, wait for in-flight with timeout
|
||||
func (s *Server) Shutdown(ctx context.Context) error {
|
||||
s.listener.Close() // stop accepting new connections
|
||||
done := make(chan struct{})
|
||||
go func() { s.active.Wait(); close(done) }()
|
||||
select {
|
||||
case <-done:
|
||||
return nil // all requests finished
|
||||
case <-ctx.Done():
|
||||
return ctx.Err() // timed out — caller decides what to do
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Anti-pattern:** Only providing one shutdown mode; not accepting a context for
|
||||
timeout control; leaking goroutines on shutdown.
|
||||
|
||||
|
||||
@@ -38,6 +38,39 @@ func (m *Mutex) Lock() {
|
||||
- **Not associated with a goroutine** — one goroutine can Lock, another can Unlock
|
||||
- **Locker interface** — abstracts over Mutex and RWMutex
|
||||
|
||||
### When to Use
|
||||
|
||||
**Triggers:**
|
||||
- Multiple goroutines read AND write the same data structure
|
||||
- You need to protect a small critical section (a few field accesses)
|
||||
- A channel-based solution would add complexity without benefit (no coordination needed, just protection)
|
||||
|
||||
**Example — before:**
|
||||
```go
|
||||
type Stats struct {
|
||||
hits int
|
||||
misses int
|
||||
}
|
||||
|
||||
func (s *Stats) RecordHit() { s.hits++ } // DATA RACE when called from multiple goroutines
|
||||
func (s *Stats) RecordMiss() { s.misses++ } // DATA RACE
|
||||
```
|
||||
|
||||
**Example — after:**
|
||||
```go
|
||||
type Stats struct {
|
||||
mu sync.Mutex
|
||||
hits int
|
||||
misses int
|
||||
}
|
||||
|
||||
func (s *Stats) RecordHit() {
|
||||
s.mu.Lock()
|
||||
defer s.mu.Unlock()
|
||||
s.hits++
|
||||
}
|
||||
```
|
||||
|
||||
### Idiomatic Usage
|
||||
|
||||
```go
|
||||
@@ -101,6 +134,40 @@ The implementation reveals a subtle guarantee: **when Do returns, f has finished
|
||||
|
||||
The `done` field is first in the struct for hot-path performance on amd64/386 (noted in comment at line 24-27).
|
||||
|
||||
### When to Use
|
||||
|
||||
**Triggers:**
|
||||
- You have expensive initialization that should happen exactly once (DB connection, config parse, compiled regex)
|
||||
- Multiple goroutines may trigger the initialization concurrently
|
||||
- You're using `var` + `if instance == nil` checks that aren't goroutine-safe
|
||||
|
||||
**Example — before:**
|
||||
```go
|
||||
var db *sql.DB
|
||||
|
||||
func GetDB() *sql.DB {
|
||||
if db == nil { // RACE: two goroutines can both see nil
|
||||
db, _ = sql.Open("postgres", connStr)
|
||||
}
|
||||
return db
|
||||
}
|
||||
```
|
||||
|
||||
**Example — after:**
|
||||
```go
|
||||
var (
|
||||
db *sql.DB
|
||||
once sync.Once
|
||||
)
|
||||
|
||||
func GetDB() *sql.DB {
|
||||
once.Do(func() {
|
||||
db, _ = sql.Open("postgres", connStr)
|
||||
})
|
||||
return db
|
||||
}
|
||||
```
|
||||
|
||||
### Idiomatic Usage
|
||||
|
||||
```go
|
||||
@@ -297,6 +364,51 @@ case result := <-work:
|
||||
}
|
||||
```
|
||||
|
||||
### When to Use
|
||||
|
||||
**Triggers:**
|
||||
- You need to broadcast "stop" to multiple goroutines simultaneously
|
||||
- A goroutine needs to select between work and cancellation
|
||||
- You're implementing graceful shutdown for a long-running service
|
||||
|
||||
**Example — before:**
|
||||
```go
|
||||
type Server struct {
|
||||
stopped bool // RACE: no synchronization
|
||||
}
|
||||
|
||||
func (s *Server) worker() {
|
||||
for {
|
||||
if s.stopped { return } // busy-polls, racy
|
||||
doWork()
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Example — after:**
|
||||
```go
|
||||
type Server struct {
|
||||
done chan struct{}
|
||||
}
|
||||
|
||||
func NewServer() *Server {
|
||||
return &Server{done: make(chan struct{})}
|
||||
}
|
||||
|
||||
func (s *Server) worker() {
|
||||
for {
|
||||
select {
|
||||
case <-s.done:
|
||||
return
|
||||
case work := <-s.workCh:
|
||||
process(work)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func (s *Server) Stop() { close(s.done) } // broadcasts to ALL workers
|
||||
```
|
||||
|
||||
### Anti-pattern
|
||||
|
||||
```go
|
||||
@@ -494,6 +606,48 @@ func generate(ctx context.Context) <-chan int {
|
||||
}
|
||||
```
|
||||
|
||||
### When to Use
|
||||
|
||||
**Triggers:**
|
||||
- You have a producer-consumer flow where the consumer's speed should limit the producer (backpressure)
|
||||
- Data flows through multiple transformation stages
|
||||
- You want to decouple stages that can run concurrently
|
||||
|
||||
**Example — before:**
|
||||
```go
|
||||
func processAll(items []string) []Result {
|
||||
var results []Result
|
||||
for _, item := range items {
|
||||
fetched := fetch(item) // sequential: fetch then transform
|
||||
results = append(results, transform(fetched))
|
||||
}
|
||||
return results
|
||||
}
|
||||
```
|
||||
|
||||
**Example — after:**
|
||||
```go
|
||||
func processAll(ctx context.Context, items []string) []Result {
|
||||
fetched := make(chan Fetched)
|
||||
go func() {
|
||||
defer close(fetched)
|
||||
for _, item := range items {
|
||||
select {
|
||||
case fetched <- fetch(item): // backpressure: blocks if transform is slow
|
||||
case <-ctx.Done():
|
||||
return
|
||||
}
|
||||
}
|
||||
}()
|
||||
|
||||
var results []Result
|
||||
for f := range fetched {
|
||||
results = append(results, transform(f))
|
||||
}
|
||||
return results
|
||||
}
|
||||
```
|
||||
|
||||
### Anti-pattern
|
||||
|
||||
```go
|
||||
|
||||
@@ -168,6 +168,35 @@ an `// Output:` comment that `go test` verifies.
|
||||
in `go doc` and pkg.go.dev alongside the relevant symbol. They teach by showing
|
||||
real, working code.
|
||||
|
||||
**When to Use**
|
||||
|
||||
**Triggers:**
|
||||
- You have a public function/type whose usage isn't obvious from the signature alone
|
||||
- Your README examples have drifted from the actual API (broken examples in docs)
|
||||
- You want examples that appear on pkg.go.dev AND are verified by `go test`
|
||||
|
||||
**Example — before:**
|
||||
```go
|
||||
// README.md (may be stale):
|
||||
// ```go
|
||||
// result := mylib.Process("input")
|
||||
// fmt.Println(result.Data)
|
||||
// ```
|
||||
// ← compiles? who knows. API changed last month.
|
||||
```
|
||||
|
||||
**Example — after:**
|
||||
```go
|
||||
// example_test.go
|
||||
func ExampleProcess() {
|
||||
result := mylib.Process("input")
|
||||
fmt.Println(result.Data)
|
||||
// Output:
|
||||
// processed: input
|
||||
}
|
||||
// ← go test verifies this compiles and produces the expected output
|
||||
```
|
||||
|
||||
**Anti-pattern:** Examples that don't compile; examples without Output comments
|
||||
(not verified); examples in README that drift from reality.
|
||||
|
||||
@@ -266,6 +295,30 @@ deprecated and explains what to use instead.
|
||||
**Why:** Recognized by tooling (go vet, staticcheck, IDEs). Provides a migration
|
||||
path without breaking backward compatibility.
|
||||
|
||||
**When to Use**
|
||||
|
||||
**Triggers:**
|
||||
- You have a better replacement for an existing function but can't remove the old one (semver)
|
||||
- Users are still calling a function that has known issues or a superior alternative
|
||||
- You want IDEs to show a strikethrough and linters to warn on usage
|
||||
|
||||
**Example — before:**
|
||||
```go
|
||||
// Just delete it? Breaks everyone's code.
|
||||
// Leave it silently? Users never learn about the better way.
|
||||
```
|
||||
|
||||
**Example — after:**
|
||||
```go
|
||||
// ParseDuration parses a duration string.
|
||||
//
|
||||
// Deprecated: Use [time.ParseDuration] instead, which handles
|
||||
// all standard duration formats.
|
||||
func ParseDuration(s string) (time.Duration, error) {
|
||||
return time.ParseDuration(s) // delegate to the replacement
|
||||
}
|
||||
```
|
||||
|
||||
**Anti-pattern:** Removing deprecated APIs (breaks semver); deprecating without
|
||||
suggesting an alternative; using non-standard deprecation markers.
|
||||
|
||||
|
||||
@@ -36,6 +36,44 @@ if err == io.EOF {
|
||||
|
||||
**Critical rule from io.EOF's doc comment**: Read must return EOF itself, **not an error wrapping EOF**, because callers test for it with `==`. This is the distinction between sentinel errors (identity-checked) and wrapped errors (tree-checked).
|
||||
|
||||
### When to Use
|
||||
|
||||
**Triggers:**
|
||||
- You have a specific, well-known failure condition callers need to check by identity
|
||||
- Multiple packages compare against the same error value (`io.EOF`, `sql.ErrNoRows`)
|
||||
- The error represents a **state** ("end of stream", "not found"), not a bug
|
||||
|
||||
**Example — before:**
|
||||
```go
|
||||
func fetchUser(id int) (*User, error) {
|
||||
row := db.QueryRow("SELECT ...")
|
||||
var u User
|
||||
err := row.Scan(&u.Name)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("user not found") // caller can't distinguish "not found" from "db down"
|
||||
}
|
||||
return &u, nil
|
||||
}
|
||||
```
|
||||
|
||||
**Example — after:**
|
||||
```go
|
||||
var ErrUserNotFound = errors.New("users: not found")
|
||||
|
||||
func fetchUser(id int) (*User, error) {
|
||||
row := db.QueryRow("SELECT ...")
|
||||
var u User
|
||||
err := row.Scan(&u.Name)
|
||||
if errors.Is(err, sql.ErrNoRows) {
|
||||
return nil, ErrUserNotFound // sentinel: callers can test with errors.Is
|
||||
}
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("fetchUser: %w", err)
|
||||
}
|
||||
return &u, nil
|
||||
}
|
||||
```
|
||||
|
||||
### Anti-pattern
|
||||
|
||||
```go
|
||||
@@ -137,6 +175,43 @@ return fmt.Errorf("open config: %w", err)
|
||||
return fmt.Errorf("open config: %v", err)
|
||||
```
|
||||
|
||||
### When to Use
|
||||
|
||||
**Triggers:**
|
||||
- You're adding context to an error before returning it up the call stack
|
||||
- The caller's error message would be meaningless without knowing *what* operation failed
|
||||
- You have a chain of function calls and want a readable error trail: `"open config: read file: permission denied"`
|
||||
|
||||
**Example — before:**
|
||||
```go
|
||||
func loadConfig(path string) (*Config, error) {
|
||||
data, err := os.ReadFile(path)
|
||||
if err != nil {
|
||||
return nil, err // caller sees "open /etc/app.conf: permission denied" — no context about WHO called ReadFile
|
||||
}
|
||||
var cfg Config
|
||||
if err := json.Unmarshal(data, &cfg); err != nil {
|
||||
return nil, err // caller can't tell if this was a read error or a parse error
|
||||
}
|
||||
return &cfg, nil
|
||||
}
|
||||
```
|
||||
|
||||
**Example — after:**
|
||||
```go
|
||||
func loadConfig(path string) (*Config, error) {
|
||||
data, err := os.ReadFile(path)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("load config: %w", err) // wraps: callers can errors.Is(err, os.ErrNotExist)
|
||||
}
|
||||
var cfg Config
|
||||
if err := json.Unmarshal(data, &cfg); err != nil {
|
||||
return nil, fmt.Errorf("load config: parse %s: %v", path, err) // %v: hides internal JSON error type
|
||||
}
|
||||
return &cfg, nil
|
||||
}
|
||||
```
|
||||
|
||||
### When to use %w vs %v
|
||||
|
||||
- **%w**: When the wrapped error is part of your API contract. Callers can depend on it.
|
||||
@@ -313,6 +388,37 @@ errs = append(errs, closeCache())
|
||||
return errors.Join(errs...) // nil if all nil
|
||||
```
|
||||
|
||||
### When to Use
|
||||
|
||||
**Triggers:**
|
||||
- You're closing/cleaning up multiple resources and each can fail independently
|
||||
- A validation function checks multiple fields and you want ALL errors, not just the first
|
||||
- You're running parallel operations and collecting errors from each
|
||||
|
||||
**Example — before:**
|
||||
```go
|
||||
func cleanup(db *sql.DB, cache *redis.Client, file *os.File) error {
|
||||
if err := db.Close(); err != nil {
|
||||
return err // stops here — cache and file leak!
|
||||
}
|
||||
if err := cache.Close(); err != nil {
|
||||
return err // file still leaks
|
||||
}
|
||||
return file.Close()
|
||||
}
|
||||
```
|
||||
|
||||
**Example — after:**
|
||||
```go
|
||||
func cleanup(db *sql.DB, cache *redis.Client, file *os.File) error {
|
||||
return errors.Join(
|
||||
db.Close(),
|
||||
cache.Close(),
|
||||
file.Close(),
|
||||
) // nil if all nil; contains all failures otherwise
|
||||
}
|
||||
```
|
||||
|
||||
### Anti-pattern
|
||||
|
||||
```go
|
||||
|
||||
@@ -31,6 +31,35 @@ type Closer interface {
|
||||
|
||||
Small interfaces are easy to implement and easy to compose. Any type can satisfy `io.Reader` by implementing a single method. This maximizes the number of types that can participate in the ecosystem — files, network connections, buffers, compressors, encryptors all satisfy `Reader`.
|
||||
|
||||
### When to Use
|
||||
|
||||
**Triggers:**
|
||||
- You're defining a function that only needs ONE capability from its argument (reading, writing, closing)
|
||||
- You want maximum reusability — many different types should be able to satisfy your requirement
|
||||
- You're tempted to create a big interface but realize most consumers only use 1-2 methods
|
||||
|
||||
**Example — before:**
|
||||
```go
|
||||
// Accepts only *os.File — can't use with buffers, HTTP bodies, test mocks
|
||||
func countLines(f *os.File) (int, error) {
|
||||
scanner := bufio.NewScanner(f)
|
||||
count := 0
|
||||
for scanner.Scan() { count++ }
|
||||
return count, scanner.Err()
|
||||
}
|
||||
```
|
||||
|
||||
**Example — after:**
|
||||
```go
|
||||
// Accepts io.Reader — works with files, HTTP bodies, strings.NewReader, gzip.Reader, etc.
|
||||
func countLines(r io.Reader) (int, error) {
|
||||
scanner := bufio.NewScanner(r)
|
||||
count := 0
|
||||
for scanner.Scan() { count++ }
|
||||
return count, scanner.Err()
|
||||
}
|
||||
```
|
||||
|
||||
### Anti-pattern
|
||||
|
||||
```go
|
||||
@@ -127,6 +156,37 @@ func TeeReader(r Reader, w Writer) Reader {
|
||||
|
||||
The return type of `LimitReader` is `Reader` (interface), but the underlying value is `*LimitedReader` (struct). Functions like `io.Copy` can type-assert to `*LimitedReader` to optimize buffer sizes (line 425).
|
||||
|
||||
### When to Use
|
||||
|
||||
**Triggers:**
|
||||
- You're writing a function/constructor that operates on a capability (reading, hashing, connecting)
|
||||
- Your return type has useful fields or methods beyond the interface contract
|
||||
- You want callers to pass anything that satisfies the contract, but return something concrete they can inspect
|
||||
|
||||
**Example — before:**
|
||||
```go
|
||||
// Too restrictive input, too vague output
|
||||
func NewLogger(f *os.File) io.Writer {
|
||||
return &logger{out: f, level: "info"} // hides SetLevel, Flush methods
|
||||
}
|
||||
```
|
||||
|
||||
**Example — after:**
|
||||
```go
|
||||
type Logger struct {
|
||||
out io.Writer
|
||||
level string
|
||||
}
|
||||
|
||||
func (l *Logger) SetLevel(lvl string) { l.level = lvl }
|
||||
func (l *Logger) Flush() error { /* ... */ }
|
||||
|
||||
// Accept interface (any io.Writer), return struct (full access)
|
||||
func NewLogger(w io.Writer) *Logger {
|
||||
return &Logger{out: w, level: "info"}
|
||||
}
|
||||
```
|
||||
|
||||
### Anti-pattern
|
||||
|
||||
```go
|
||||
@@ -218,6 +278,45 @@ func (f HandlerFunc) ServeHTTP(w ResponseWriter, r *Request) {
|
||||
|
||||
This bridges functions and interfaces. Any function with the right signature becomes a `Handler` via `HandlerFunc(myFunc)`. You get the simplicity of functions with the composability of interfaces.
|
||||
|
||||
### When to Use
|
||||
|
||||
**Triggers:**
|
||||
- You have an interface with a single method and users frequently implement it with a bare function
|
||||
- You want to accept both struct-based and function-based implementations of the same behavior
|
||||
- Requiring a struct definition for simple cases feels like boilerplate
|
||||
|
||||
**Example — before:**
|
||||
```go
|
||||
type Processor interface {
|
||||
Process(data []byte) error
|
||||
}
|
||||
|
||||
// User must create a whole struct just to use a function
|
||||
type upperProcessor struct{}
|
||||
func (u upperProcessor) Process(data []byte) error {
|
||||
fmt.Println(strings.ToUpper(string(data)))
|
||||
return nil
|
||||
}
|
||||
```
|
||||
|
||||
**Example — after:**
|
||||
```go
|
||||
type Processor interface {
|
||||
Process(data []byte) error
|
||||
}
|
||||
|
||||
// Adapter: any function with the right signature becomes a Processor
|
||||
type ProcessorFunc func([]byte) error
|
||||
|
||||
func (f ProcessorFunc) Process(data []byte) error { return f(data) }
|
||||
|
||||
// Now users can write:
|
||||
pipeline.Use(ProcessorFunc(func(data []byte) error {
|
||||
fmt.Println(strings.ToUpper(string(data)))
|
||||
return nil
|
||||
}))
|
||||
```
|
||||
|
||||
### Anti-pattern
|
||||
|
||||
```go
|
||||
@@ -259,6 +358,42 @@ if flusher, ok := w.(Flusher); ok {
|
||||
|
||||
Not every `ResponseWriter` supports flushing or hijacking (HTTP/2 doesn't support Hijacker). Instead of bloating the main interface, optional capabilities are separate interfaces checked at runtime. This keeps the core interface small while allowing progressive enhancement.
|
||||
|
||||
### When to Use
|
||||
|
||||
**Triggers:**
|
||||
- Some implementations support a capability but others don't (flushing, hijacking, seeking)
|
||||
- You want to keep the core interface small but allow optimizations when available
|
||||
- You're writing middleware that should enhance behavior when possible, not require it
|
||||
|
||||
**Example — before:**
|
||||
```go
|
||||
// Forces ALL stores to implement caching, even simple ones
|
||||
type Store interface {
|
||||
Get(key string) ([]byte, error)
|
||||
Set(key string, val []byte) error
|
||||
InvalidateCache() error // not all stores have a cache!
|
||||
}
|
||||
```
|
||||
|
||||
**Example — after:**
|
||||
```go
|
||||
type Store interface {
|
||||
Get(key string) ([]byte, error)
|
||||
Set(key string, val []byte) error
|
||||
}
|
||||
|
||||
// Optional capability — check at runtime
|
||||
type Cacheable interface {
|
||||
InvalidateCache() error
|
||||
}
|
||||
|
||||
func refreshAll(s Store) {
|
||||
if c, ok := s.(Cacheable); ok {
|
||||
c.InvalidateCache() // only if supported
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
### Anti-pattern
|
||||
|
||||
```go
|
||||
|
||||
@@ -117,6 +117,32 @@ Packages under `internal/` can only be imported by code rooted at the parent of
|
||||
- `net/http/internal/ascii` → importable by `net/http` and children
|
||||
- NOT importable by `net/url` or any other package
|
||||
|
||||
### When to Use
|
||||
|
||||
**Triggers:**
|
||||
- You have helper code shared between sub-packages but NOT part of your public API
|
||||
- You're tempted to export a function "just for testing" — put it in `internal/` instead
|
||||
- Your package has grown and you want to split it without committing to new public APIs
|
||||
|
||||
**Example — before:**
|
||||
```go
|
||||
// pkg/mylib/helpers.go — exported just so pkg/mylib/sub can use it
|
||||
package mylib
|
||||
|
||||
func ParseInternalFormat(s string) Thing { ... } // now anyone can depend on this!
|
||||
```
|
||||
|
||||
**Example — after:**
|
||||
```go
|
||||
// pkg/mylib/internal/parse/parse.go
|
||||
package parse
|
||||
|
||||
func InternalFormat(s string) Thing { ... } // only importable by pkg/mylib and children
|
||||
|
||||
// pkg/mylib/sub/handler.go
|
||||
import "pkg/mylib/internal/parse" // ✓ allowed
|
||||
```
|
||||
|
||||
### Anti-pattern
|
||||
|
||||
```go
|
||||
@@ -192,6 +218,34 @@ The stdlib uses `init()` for:
|
||||
3. Keep them short
|
||||
4. Prefer explicit initialization in `main()` when possible
|
||||
|
||||
### When to Use
|
||||
|
||||
**Triggers:**
|
||||
- You're writing a driver or plugin that needs to register itself with a central registry on import
|
||||
- The registration is side-effect-only (no return value, can't fail)
|
||||
- You want `import _ "mydb/driver"` to make the driver available without explicit setup
|
||||
|
||||
**Example — before:**
|
||||
```go
|
||||
// main.go — user must manually register every driver
|
||||
func main() {
|
||||
postgres.Register() // easy to forget
|
||||
mysql.Register() // order matters?
|
||||
sqlite.Register()
|
||||
}
|
||||
```
|
||||
|
||||
**Example — after:**
|
||||
```go
|
||||
// postgres/driver.go
|
||||
func init() {
|
||||
sql.Register("postgres", &Driver{}) // auto-registers on import
|
||||
}
|
||||
|
||||
// main.go — import for side-effect
|
||||
import _ "github.com/lib/pq" // driver registers itself
|
||||
```
|
||||
|
||||
### Anti-pattern
|
||||
|
||||
```go
|
||||
@@ -404,6 +458,34 @@ type contextKey struct {
|
||||
- **Type-safe accessors** avoid repeated type assertions
|
||||
- **Pointer-based keys** guarantee uniqueness
|
||||
|
||||
### When to Use
|
||||
|
||||
**Triggers:**
|
||||
- You need to pass request-scoped metadata through a call chain (user ID, trace ID, auth token)
|
||||
- The data crosses package boundaries and isn't appropriate as a function parameter
|
||||
- You want type safety — only your package should read/write its context values
|
||||
|
||||
**Example — before:**
|
||||
```go
|
||||
// String keys — any package can collide or access your values
|
||||
ctx = context.WithValue(ctx, "userID", 42)
|
||||
uid := ctx.Value("userID").(int) // panics if wrong type or missing
|
||||
```
|
||||
|
||||
**Example — after:**
|
||||
```go
|
||||
type ctxKey struct{}
|
||||
|
||||
func WithUserID(ctx context.Context, id int) context.Context {
|
||||
return context.WithValue(ctx, ctxKey{}, id)
|
||||
}
|
||||
|
||||
func UserID(ctx context.Context) (int, bool) {
|
||||
id, ok := ctx.Value(ctxKey{}).(int)
|
||||
return id, ok
|
||||
}
|
||||
```
|
||||
|
||||
### Anti-pattern
|
||||
|
||||
```go
|
||||
|
||||
@@ -13,6 +13,39 @@ explicit initialization. Nil fields fall back to sensible defaults at method cal
|
||||
self-documenting about its defaults. Users can write `var c http.Client` and start
|
||||
making requests.
|
||||
|
||||
**When to Use**
|
||||
|
||||
**Triggers:**
|
||||
- You're designing a type where the "empty" or "default" state is meaningful and safe
|
||||
- Users should be able to write `var x MyType` and immediately call methods
|
||||
- Your struct's nil/zero fields can fall back to sensible defaults at call time
|
||||
|
||||
**Example — before:**
|
||||
```go
|
||||
type Cache struct {
|
||||
store map[string][]byte
|
||||
ttl time.Duration
|
||||
}
|
||||
|
||||
// Panics on zero value — store is nil!
|
||||
func (c *Cache) Set(k string, v []byte) { c.store[k] = v }
|
||||
```
|
||||
|
||||
**Example — after:**
|
||||
```go
|
||||
type Cache struct {
|
||||
store map[string][]byte
|
||||
ttl time.Duration // zero means no expiry
|
||||
}
|
||||
|
||||
func (c *Cache) Set(k string, v []byte) {
|
||||
if c.store == nil {
|
||||
c.store = make(map[string][]byte) // lazy init on first use
|
||||
}
|
||||
c.store[k] = v
|
||||
}
|
||||
```
|
||||
|
||||
**Anti-pattern:** Requiring a constructor for basic use; panicking on zero-value use;
|
||||
requiring all fields be set before the type is functional.
|
||||
|
||||
@@ -116,6 +149,39 @@ value alone.
|
||||
clearly communicates what's required. The constructor can set internal invariants
|
||||
(buffer sizes, split functions) that users shouldn't need to know about.
|
||||
|
||||
**When to Use**
|
||||
|
||||
**Triggers:**
|
||||
- Your type has mandatory dependencies that can't be expressed as zero values (an `io.Reader`, a DB connection)
|
||||
- Internal invariants must be set up (buffer allocation, goroutine start)
|
||||
- The type isn't useful without initialization (unlike `sync.Mutex` or `bytes.Buffer`)
|
||||
|
||||
**Example — before:**
|
||||
```go
|
||||
type Parser struct {
|
||||
lexer *Lexer
|
||||
buf []Token
|
||||
maxDepth int
|
||||
}
|
||||
|
||||
// User must know about all internal state:
|
||||
p := &Parser{lexer: NewLexer(input), buf: make([]Token, 0, 64), maxDepth: 100}
|
||||
```
|
||||
|
||||
**Example — after:**
|
||||
```go
|
||||
func NewParser(input io.Reader) *Parser {
|
||||
return &Parser{
|
||||
lexer: NewLexer(input),
|
||||
buf: make([]Token, 0, 64),
|
||||
maxDepth: 100,
|
||||
}
|
||||
}
|
||||
|
||||
// User writes:
|
||||
p := NewParser(file)
|
||||
```
|
||||
|
||||
**Anti-pattern:** Forcing users to manually set unexported fields; having a constructor
|
||||
that takes 10 optional parameters (use config struct instead); requiring New when
|
||||
zero value would suffice.
|
||||
@@ -205,6 +271,35 @@ configuration knobs. Nil/zero values always mean "use the default".
|
||||
construct partially; serializable; the zero value works. This is Go's primary
|
||||
configuration pattern (preferred over functional options in the stdlib).
|
||||
|
||||
**When to Use**
|
||||
|
||||
**Triggers:**
|
||||
- Your constructor has 4+ optional parameters that would make a function signature unwieldy
|
||||
- You want users to see all options in one place with godoc documentation
|
||||
- Zero/nil values should mean "use the default" — no required fields beyond what the constructor demands
|
||||
|
||||
**Example — before:**
|
||||
```go
|
||||
// 7 parameters — impossible to remember the order
|
||||
func NewServer(addr string, handler http.Handler, readTimeout, writeTimeout time.Duration,
|
||||
maxConns int, logger *log.Logger, tlsConfig *tls.Config) *Server { ... }
|
||||
```
|
||||
|
||||
**Example — after:**
|
||||
```go
|
||||
type ServerConfig struct {
|
||||
Addr string // ":8080" if empty
|
||||
Handler http.Handler // http.DefaultServeMux if nil
|
||||
ReadTimeout time.Duration // zero means no timeout
|
||||
WriteTimeout time.Duration // zero means no timeout
|
||||
MaxConns int // 1000 if zero
|
||||
Logger *log.Logger // log.Default() if nil
|
||||
TLSConfig *tls.Config // plain HTTP if nil
|
||||
}
|
||||
|
||||
func NewServer(cfg ServerConfig) *Server { ... }
|
||||
```
|
||||
|
||||
**Anti-pattern:** Undocumented fields; requiring all fields set; using sentinel values
|
||||
other than zero/nil for defaults; providing setters when direct assignment works.
|
||||
|
||||
|
||||
@@ -110,6 +110,33 @@ build time.
|
||||
**Why:** Catches interface drift at compile time without creating an instance. The
|
||||
blank identifier discards the value — this is purely a static assertion.
|
||||
|
||||
**When to Use**
|
||||
|
||||
**Triggers:**
|
||||
- You've defined a type that MUST satisfy an interface (implements `io.Reader`, `http.Handler`, etc.)
|
||||
- You want a compile-time guarantee that catches drift when you add/remove methods
|
||||
- You're writing a package with multiple types implementing the same interface
|
||||
|
||||
**Example — before:**
|
||||
```go
|
||||
type MyWriter struct{ buf bytes.Buffer }
|
||||
|
||||
func (w *MyWriter) Write(p []byte) (int, error) { return w.buf.Write(p) }
|
||||
|
||||
// Months later, someone renames Write to WriteBuf...
|
||||
// No compile error — only discovered at runtime when passed as io.Writer
|
||||
```
|
||||
|
||||
**Example — after:**
|
||||
```go
|
||||
// Compile-time check: if MyWriter stops implementing io.Writer, this fails to build
|
||||
var _ io.Writer = (*MyWriter)(nil)
|
||||
|
||||
type MyWriter struct{ buf bytes.Buffer }
|
||||
|
||||
func (w *MyWriter) Write(p []byte) (int, error) { return w.buf.Write(p) }
|
||||
```
|
||||
|
||||
**Anti-pattern:** Relying on tests to catch interface conformance; skipping the check
|
||||
and discovering the mismatch at runtime; using reflection.
|
||||
|
||||
@@ -312,6 +339,39 @@ are exhaustively listed together.
|
||||
**Why:** Type safety (can't accidentally pass an `os.Flag` where a `crypto.Hash` is
|
||||
expected). `iota` eliminates magic numbers. Grouping makes the full set visible.
|
||||
|
||||
**When to Use**
|
||||
|
||||
**Triggers:**
|
||||
- You have a set of related values that represent distinct states or options (status codes, modes, categories)
|
||||
- Raw integers would be meaningless to readers (`SetMode(3)` vs `SetMode(ModeAsync)`)
|
||||
- You want the type system to prevent passing a "color" where a "direction" is expected
|
||||
|
||||
**Example — before:**
|
||||
```go
|
||||
func SetLogLevel(level int) { ... }
|
||||
|
||||
// Caller:
|
||||
SetLogLevel(3) // what does 3 mean?!
|
||||
SetLogLevel(-1) // valid? who knows
|
||||
```
|
||||
|
||||
**Example — after:**
|
||||
```go
|
||||
type LogLevel int
|
||||
|
||||
const (
|
||||
LevelDebug LogLevel = iota
|
||||
LevelInfo
|
||||
LevelWarn
|
||||
LevelError
|
||||
)
|
||||
|
||||
func SetLogLevel(level LogLevel) { ... }
|
||||
|
||||
// Caller:
|
||||
SetLogLevel(LevelWarn) // self-documenting
|
||||
```
|
||||
|
||||
**Anti-pattern:** Untyped numeric constants; separate `const` declarations for related
|
||||
values; using raw integers in function signatures.
|
||||
|
||||
@@ -382,6 +442,34 @@ accidental mixing with raw int64 values.
|
||||
nanoseconds where seconds are expected. Methods provide conversion and formatting.
|
||||
Constants like `time.Second` make intent clear.
|
||||
|
||||
**When to Use**
|
||||
|
||||
**Triggers:**
|
||||
- A primitive type (`int`, `string`, `float64`) has a specific **semantic meaning** in your domain
|
||||
- You want to attach methods (formatting, validation, arithmetic) to the value
|
||||
- You've seen bugs from accidentally mixing units (`int` could be seconds, milliseconds, or nanoseconds)
|
||||
|
||||
**Example — before:**
|
||||
```go
|
||||
func SetTimeout(ms int) { ... } // is this milliseconds? seconds?
|
||||
func SetRetries(n int) { ... } // can't accidentally swap these... or CAN you?
|
||||
|
||||
SetTimeout(5) // 5 what?
|
||||
SetRetries(500) // oops, swapped arguments — compiles fine!
|
||||
```
|
||||
|
||||
**Example — after:**
|
||||
```go
|
||||
type Timeout time.Duration
|
||||
type RetryCount int
|
||||
|
||||
func SetTimeout(t Timeout) { ... }
|
||||
func SetRetries(n RetryCount) { ... }
|
||||
|
||||
SetTimeout(Timeout(5 * time.Second)) // explicit units
|
||||
SetRetries(RetryCount(3)) // can't swap — different types
|
||||
```
|
||||
|
||||
**Anti-pattern:** Using raw `int64` for durations; accepting `int` parameters for
|
||||
time intervals; mixing units (milliseconds in one place, seconds in another).
|
||||
|
||||
|
||||
@@ -16,6 +16,55 @@ The canonical Go test style. Every Go stdlib test file uses this pattern.
|
||||
|
||||
**Why:** Eliminates repetition, makes adding cases trivial, keeps the assertion logic in one place. Every test case gets the same verification path — no "special" cases hidden in different code paths.
|
||||
|
||||
**When to Use**
|
||||
|
||||
**Triggers:**
|
||||
- You're testing a function with many input/output combinations
|
||||
- You're copy-pasting test functions that differ by one or two values
|
||||
- Adding a new test case requires duplicating 10+ lines of setup/assertion code
|
||||
|
||||
**Example — before:**
|
||||
```go
|
||||
func TestParseSize(t *testing.T) {
|
||||
result1, err1 := ParseSize("10MB")
|
||||
if err1 != nil || result1 != 10_000_000 { t.Error("10MB failed") }
|
||||
|
||||
result2, err2 := ParseSize("1GB")
|
||||
if err2 != nil || result2 != 1_000_000_000 { t.Error("1GB failed") }
|
||||
|
||||
result3, err3 := ParseSize("invalid")
|
||||
if err3 == nil { t.Error("invalid should fail") }
|
||||
// ... 20 more copy-pasted blocks
|
||||
}
|
||||
```
|
||||
|
||||
**Example — after:**
|
||||
```go
|
||||
func TestParseSize(t *testing.T) {
|
||||
tests := []struct {
|
||||
input string
|
||||
want int64
|
||||
wantErr bool
|
||||
}{
|
||||
{"10MB", 10_000_000, false},
|
||||
{"1GB", 1_000_000_000, false},
|
||||
{"invalid", 0, true},
|
||||
{"0B", 0, false},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.input, func(t *testing.T) {
|
||||
got, err := ParseSize(tt.input)
|
||||
if (err != nil) != tt.wantErr {
|
||||
t.Fatalf("ParseSize(%q) error = %v, wantErr %v", tt.input, err, tt.wantErr)
|
||||
}
|
||||
if got != tt.want {
|
||||
t.Errorf("ParseSize(%q) = %d, want %d", tt.input, got, tt.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Anti-pattern:** Writing individual assertions for each case, or copy-pasting test functions that differ by one input.
|
||||
|
||||
**Code example (stdlib):**
|
||||
@@ -266,6 +315,48 @@ ServeFile(w, r, "testdata/file")
|
||||
3. Golden files serve as documentation of expected behavior.
|
||||
4. Reviewers can see exactly what output changed in diffs.
|
||||
|
||||
**When to Use**
|
||||
|
||||
**Triggers:**
|
||||
- Your function produces complex multi-line output (formatted code, HTML, JSON, error messages)
|
||||
- Expected output would be 20+ lines if inlined in the test — unreadable
|
||||
- Output changes intentionally sometimes and you need a quick way to approve the new version
|
||||
|
||||
**Example — before:**
|
||||
```go
|
||||
func TestRenderTemplate(t *testing.T) {
|
||||
got := renderHTML(data)
|
||||
want := `<!DOCTYPE html>
|
||||
<html>
|
||||
<head><title>Hello</title></head>
|
||||
<body>
|
||||
<h1>Welcome, Alice</h1>
|
||||
<p>You have 3 messages.</p>
|
||||
</body>
|
||||
</html>` // 8 lines inline — and this is a SIMPLE template
|
||||
if got != want { t.Errorf("mismatch") }
|
||||
}
|
||||
```
|
||||
|
||||
**Example — after:**
|
||||
```go
|
||||
var update = flag.Bool("update", false, "update golden files")
|
||||
|
||||
func TestRenderTemplate(t *testing.T) {
|
||||
got := renderHTML(data)
|
||||
golden := filepath.Join("testdata", t.Name()+".golden")
|
||||
if *update {
|
||||
os.WriteFile(golden, []byte(got), 0644)
|
||||
return
|
||||
}
|
||||
want, _ := os.ReadFile(golden)
|
||||
if got != string(want) {
|
||||
t.Errorf("output mismatch; run with -update to accept new output")
|
||||
}
|
||||
}
|
||||
// Golden file lives at testdata/TestRenderTemplate.golden
|
||||
```
|
||||
|
||||
**Anti-pattern:** Comparing against inline expected strings that span 50+ lines, or manually constructing expected output.
|
||||
|
||||
**Code example (stdlib):**
|
||||
@@ -319,6 +410,38 @@ func TestRewrite(t *testing.T) {
|
||||
|
||||
**Why:** Fast, no network, no port allocation, no goroutines. Perfect for unit testing individual handlers in isolation.
|
||||
|
||||
**When to Use**
|
||||
|
||||
**Triggers:**
|
||||
- You're testing HTTP handler logic (status codes, headers, response body) in isolation
|
||||
- You don't need real TCP connections, TLS, or routing
|
||||
- Your test should run in <1ms, not wait for port binding
|
||||
|
||||
**Example — before:**
|
||||
```go
|
||||
func TestHealthHandler(t *testing.T) {
|
||||
srv := httptest.NewServer(http.HandlerFunc(healthHandler))
|
||||
defer srv.Close()
|
||||
resp, _ := http.Get(srv.URL + "/health") // real TCP connection — slow
|
||||
if resp.StatusCode != 200 { t.Fatal("not healthy") }
|
||||
}
|
||||
```
|
||||
|
||||
**Example — after:**
|
||||
```go
|
||||
func TestHealthHandler(t *testing.T) {
|
||||
req := httptest.NewRequest("GET", "/health", nil)
|
||||
rec := httptest.NewRecorder()
|
||||
healthHandler(rec, req) // direct call — no network
|
||||
if rec.Code != 200 {
|
||||
t.Fatalf("got status %d, want 200", rec.Code)
|
||||
}
|
||||
if rec.Body.String() != "ok" {
|
||||
t.Errorf("body = %q, want %q", rec.Body.String(), "ok")
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
**Anti-pattern:** Spinning up a full server to test handler logic that doesn't need networking.
|
||||
|
||||
**Code example (stdlib):**
|
||||
|
||||
Reference in New Issue
Block a user