Writing Better Go: Lessons from 10 Code Reviews

Why do Go developers obsess over variable names, error handling, and interfaces, even when your approach does the job, too? Whether gently or not so gently, they'll remind you there's a right way to write Go.

In this talk, I'll walk through frequent code review comments to show what looks like nitpicking is a window into Go's design philosophy. These aren't arbitrary style debates; they're about writing clear, deliberate, and unapologetically pragmatic Go. You'll leave with a better understanding of the "why" behind the rules and how embracing them can sharpen your code.

GoLab 2025, Florence

Transcript

1. Writing Better Go Lessons from 10 Code Reviews Konrad Reiche GoLab 2025
2. “Rename this variable.”
3. “Indent this line.”
4. “Did you even install gofmt?”
5. “Don’t put a blank line between assigning an error and checking it.”
6. Why Bother? The View From the Review Queue Small style comments often resurface as real production issues. Code review is where we learn, whether we want to or not. Reviewing hundreds of pull requests turned repetition into patterns and patterns into guidance.
7. Why Bother? The View From the Review Queue Code Written per Week Small style comments often resurface as real production issues. Code review is where we learn, whether we want to or not. Reviewing hundreds of pull requests turned repetition into patterns and patterns into guidance. Code Reviewed per Week vs.
8. 01 Handle Errors
9. Handle Errors BAD: Silently Discarding result, _ := pickRandom(input)
10. Handle Errors BAD: Silently Discarding result, _ := pickRandom(input) func pickRandom(input []string) (string, error)
11. Handle Errors BAD: Silently Ignoring result, err := pickRandom(input) if err != nil { }
12. Handle Errors BAD: Swallowing the Error result, err := pickRandom(input) if err != nil { return nil }
13. Handle Errors Good: Checking and Handling the Error result, err := pickRandom(input) if err != nil { return err }
14. Handle Errors result, err := pickRandom(input) if err != nil { pickRandomFailures.Inc() return nil } Good: Checking and Handling the Error
15. Handle Errors result, err := pickRandom(input) if err != nil { slog.Error("pickRandom failed", "error", err) return nil } Good: Checking and Handling the Error
16. Handle Errors BAD: Double Reporting result, err := pickRandom(input) if err != nil { slog.Error("pickRandom failed", "error", err) return err }
17. Handle Errors BAD: Double Reporting result, err := pickRandom(input) if err != nil { slog.Error("pickRandom failed", "error", err) return err } Log it, or return it — but not both.
18. Handle Errors Good: Checking and Handling the Error + Contexualizing result, err := pickRandom(input) if err != nil { return fmt.Errorf("pickRandom failed: %w", err) }
19. Handle Errors resp, err := http.DefaultClient.Do(req) if err != nil { return resp, err }
20. func fetch(req *http.Request) (*http.Response, error) { resp, err := http.DefaultClient.Do(req) if err != nil { return resp, err } return resp, nil } func main() { invalid := &http.Request{} resp, err := fetch(invalid) if err != nil { slog.Error("fetch failed", "error", err) } slog.Info(resp.Status) }
21. panic: runtime error: invalid memory address or nil pointer dereference [signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x6461ca]
22. Handle Errors BAD: Ambiguous Contract resp, err := http.DefaultClient.Do(req) if err != nil { return resp, err }
23. return result, nil ✅ Good The result is valid and safe to use. return nil, err ✅ Good The result is invalid; handle the error. return nil, nil ❌ Bad Ambiguous case forces extra nil checks. return result, err ❌ Bad Unclear which value to trust. Handle Errors Optimize for the Caller
24. return result, nil ✅ Good The result is valid and safe to use. return nil, err ✅ Good The result is invalid; handle the error. return nil, nil ❌ Bad Ambiguous case forces extra nil checks. return result, err ⚠ Bad Unclear which value to trust. Handle Errors Optimize for the Caller Unless you need to return partial results; document explicitly.
25. 02 Adding Interfaces Too Soon
26. Adding Interfaces Too Soon Two Common Misuses • Premature Abstraction
27. Adding Interfaces Too Soon Two Common Misuses • Premature Abstraction • Support Testing
28. Adding Interfaces Too Soon Two Common Misuses • Premature Abstraction Introduced prematurely by following object-oriented patterns from languages like Java. While well-intentioned, this approach adds unnecessary complexity early in the development process. • Support Testing
29. Adding Interfaces Too Soon Premature Abstraction type EligibilityService struct { catalog *product.Catalog } func (e *EligibilityService) IsEligible( ctx context.Context, userID string, productID string, ) (bool, error) { // ... }
30. package cache import ( "context" ) type Cache[T any] interface { Get(ctx context.Context, key string) (*T, error) Set(ctx context.Context, key string, value T) error }
31. Adding Interfaces Too Soon Premature Abstraction type EligibilityService struct { cache cache.Cache[model.Product] catalog *product.Catalog } func (e *EligibilityService) IsEligible( ctx context.Context, userID string, productID string, ) (bool, error) { // ... }
32. Adding Interfaces Too Soon Premature Abstraction type EligibilityService struct { cache cache.Cache[model.Product] catalog *product.Catalog } 👆
33. package cache import ( "container/heap" "container/list" "context" "sync" ) type Cache[T any] interface { Get(ctx context.Context, key string) (*T, error) Set(ctx context.Context, key string, value T) error } type LFU[T any] struct { size int mu sync.RWMutex data map[string]*lfuItem[T] heap *MinHeap[T] }
34. package cache import ( "container/heap" "container/list" "context" "sync" ) type Cache[T any] interface { Get(ctx context.Context, key string) (*T, error) Set(ctx context.Context, key string, value T) error } type LRU[T any] struct { size int mu sync.RWMutex data map[string]*lruItem[T] queue *list.List }
35. package cache import ( "container/heap" "container/list" "context" "sync" ) type Cache[T any] interface { Get(ctx context.Context, key string) (*T, error) Set(ctx context.Context, key string, value T) error }
36. Adding Interfaces Too Soon Premature Abstraction cache/ ├── cache.go ├── heap.go ├── ... └── lfu.go
37. Adding Interfaces Too Soon Premature Abstraction cache/ ├── cache.go ├── heap.go ├── ... ├── lru.go └── lfu.go
38. Adding Interfaces Too Soon Premature Abstraction cache/ └── cache.go
39. Adding Interfaces Too Soon Premature Abstraction type EligibilityService struct { cache cache.Cache[model.Product] catalog *product.Catalog } func (e *EligibilityService) IsEligible( ctx context.Context, userID string, productID string, ) (bool, error) { // ... }
40. Adding Interfaces Too Soon Premature Abstraction func NewEligibilityService( catalog *product.Catalog, cache cache.Cache[*model.Product], ) *EligibilityService { return &EligibilityService{ catalog: catalog, catalogCache: catalogCache, } }
41. Adding Interfaces Too Soon Premature Abstraction type EligibilityService struct { cache cache.Cache[model.Product] catalog *product.Catalog }
42. Adding Interfaces Too Soon Premature Abstraction type EligibilityService struct { cache *cache.LFU[model.Product] catalog *product.Catalog }
43. Adding Interfaces Too Soon Premature Abstraction type EligibilityService struct { cache *cache.LFU[model.Product] catalog *product.Catalog } Ask yourself if you really need multiple implementations before adding the extra layer of indirection.
44. Adding Interfaces Too Soon Two Common Misuses • Premature Abstraction Introduced prematurely by following object-oriented patterns from languages like Java. While well-intentioned, this approach adds unnecessary complexity early in the development process. • Support Testing
45. Adding Interfaces Too Soon Two Common Misuses • Premature Abstraction Introduced prematurely by following object-oriented patterns from languages like Java. While well-intentioned, this approach adds unnecessary complexity early in the development process. • Support Testing A practice that relies heavily on mocking dependencies that unblocks your productivity in the short term, but weakens the expressiveness of your types and reduces code readability in the long run.
46. Adding Interfaces Too Soon Support Testing type EligibilityService struct { cache *cache.LFU[model.Product] catalog *product.Catalog }
47. Adding Interfaces Too Soon Support Testing type EligibilityService struct { cache *cache.LFU[model.Product] catalog *product.Catalog userService *dep.UserService }
48. func TestService_IsEligible(t *testing.T) { catalog := product.NewCatalog() svc := New(userService, catalog) result, err := svc.IsEligible(t.Context(), "test_user_1", "ad-free") if err != nil { t.Fatal(err) } if got, want := result, true; got != want { t.Errorf("got %t, want: %t", got, want) } }
49. func TestService_IsEligible(t *testing.T) { userService := ??? catalog := product.NewCatalog() svc := New(userService, catalog) result, err := svc.IsEligible(t.Context(), "test_user_1", "ad-free") if err != nil { t.Fatal(err) } if got, want := result, true; got != want { t.Errorf("got %t, want: %t", got, want) } }
50. Adding Interfaces Too Soon Support Testing type EligibilityService struct { cache *cache.LFU[model.Product] catalog *product.Catalog userService *dep.UserService }
51. Adding Interfaces Too Soon Support Testing type EligibilityService struct { cache *cache.LFU[model.Product] catalog *product.Catalog userService userService } type userService interface { GetUser(context.Context, string) (*dep.UserResponse, error) }
52. type mockUserService struct { getUser func(ctx context.Context, userID string) (*dep.UserResponse, error) } func (m *mockUserService) GetUser( ctx context.Context, userID string, ) (*dep.UserResponse, error) { if m.getUser != nil { return m.getUser(ctx, userID) } return nil, errors.New("mockUserService.GetUser: not implemented") }
53. func TestService_IsEligible(t *testing.T) { catalog := product.NewCatalog() svc := New(userService, catalog) result, err := svc.IsEligible(t.Context(), "test_user_1", "ad-free") if err != nil { t.Fatal(err) } if got, want := result, true; got != want { t.Errorf("got %t, want: %t", got, want) } }
54. func TestService_IsEligible(t *testing.T) { catalog := product.NewCatalog() svc := New(userService, catalog) result, err := svc.IsEligible(t.Context(), "test_user_1", "ad-free") if err != nil { t.Fatal(err) } if got, want := result, true; got != want { t.Errorf("got %t, want: %t", got, want) } }
55. func TestService_IsEligible(t *testing.T) { userService := &mockUserService{ getUser: func(context.Context, string) (*dep.UserResponse, error) { return &userpb.GetUserResponse{ Id: "test_user_1", Subscription: 1, }, nil }, } catalog := product.NewCatalog() svc := New(userService, catalog) result, err := svc.IsEligible(t.Context(), "test_user_1", "ad-free") if err != nil { t.Fatal(err) } if got, want := result, true; got != want { t.Errorf("got %t, want: %t", got, want) } }
56. func TestService_IsEligible(t *testing.T) { userService := &mockUserService{ getUser: func(context.Context, string) (*dep.UserResponse, error) { return &userpb.GetUserResponse{ Id: "test_user_1", Subscription: 1, }, nil }, } catalog := product.NewCatalog() svc := New(userService, catalog) result, err := svc.IsEligible(t.Context(), "test_user_1", "ad-free") if err != nil { t.Fatal(err) } if got, want := result, true; got != want { t.Errorf("got %t, want: %t", got, want) } } Service client wrapper package has 0% test coverage.
57. dep └── user_service.go
58. dep ├── deptest └── user_service.go
59. dep ├── deptest │ ├── deptest.go │ └── fakeuserservice │ └── fakeuserservice.go └── user_service.go
60. type Service struct { subscriptionByUser map[string]int userpb.UnimplementedUserServiceServer } func New(tb testing.TB, opts ...func(*options)) *dep.UserService { options := options{ subscriptionByUser: make(map[string]int), } for _, opt := range opts { opt(&options) } svc := &Service{ subscriptionByUser: options.subscriptionByUser, } return deptest.NewFake(tb, svc, dep.NewUserService) }
61. func (s *Service) GetUser( ctx context.Context, req *userpb.GetUserRequest, ) (*userpb.GetUserResponse, error) { subscription, ok := s.subscriptionByUser[req.Id] if !ok { return nil, errors.New("user not found") } return &userpb.GetUserResponse{ Id: req.Id, Subscription: int64(subscription), }, nil } func (s *Service) RegisterOn(srv *grpc.Server) { userpb.RegisterUserServiceServer(srv, s) }
62. type options struct { subscriptionByUser map[string]int } func WithUserSubscriptions(subscriptionByUser map[string]int) func(*options) { return func(options *options) { options.subscriptionByUser = subscriptionByUser } } func WithUserSubscription(userID string, subscription int) func(*options) { return func(options *options) { options.subscriptionByUser[userID] = subscription } }
63. func NewFake[T any]( tb testing.TB, svc registerer, newServiceClient func(addr string, opts ...grpc.DialOption) (T, error), opts ...grpc.DialOption, ) T { tb.Helper() lis := bufconn.Listen(1024 * 1024) srv := grpc.NewServer() svc.RegisterOn(srv) go func() { if err := srv.Serve(lis); err != nil { tb.Error(err) } }() tb.Cleanup(srv.Stop)
64. bufDialer := func(ctx context.Context, addr string) (net.Conn, error) { return lis.DialContext(ctx) } opts = append([]grpc.DialOption{ grpc.WithContextDialer(bufDialer), grpc.WithTransportCredentials(insecure.NewCredentials()), }, opts...) client, err := newServiceClient( "passthrough:bufnet", opts..., ) if err != nil { tb.Fatal(err) } return client }
65. func TestService_IsEligible(t *testing.T) { userService := &mockUserService{ getUser: func(context.Context, string) (*dep.UserResponse, error) { return &userpb.GetUserResponse{ Id: "test_user_1", Subscription: 1, }, nil }, } catalog := product.NewCatalog() svc := New(userService, catalog) result, err := svc.IsEligible(t.Context(), "test_user_1", "ad-free") if err != nil { t.Fatal(err) } if got, want := result, true; got != want { t.Errorf("got %t, want: %t", got, want) } }
66. func TestService_IsEligible(t *testing.T) { userService := fakeuserservice.New( t, fakeuserservice.WithUserSubscription("test_user_1", 1), ) catalog := product.NewCatalog() svc := New(userService, catalog) result, err := svc.IsEligible(t.Context(), "test_user_1", "ad-free") if err != nil { t.Fatal(err) } if got, want := result, true; got != want { t.Errorf("got %t, want: %t", got, want) } }
67. func TestService_IsEligible(t *testing.T) { userService := fakeuserservice.New( t, fakeuserservice.WithUserSubscription("test_user_1", 1), ) catalog := product.NewCatalog() svc := New(userService, catalog) result, err := svc.IsEligible(t.Context(), "test_user_1", "ad-free") if err != nil { t.Fatal(err) } if got, want := result, true; got != want { t.Errorf("got %t, want: %t", got, want) } }
68. Adding Interfaces Too Soon Support Testing type EligibilityService struct { cache *cache.LFU[model.Product] catalog *product.Catalog userService userService } type userService interface { GetUser(context.Context, string) (*dep.UserResponse, error) }
69. Adding Interfaces Too Soon Support Testing type EligibilityService struct { cache *cache.LFU[model.Product] catalog *product.Catalog userService *dep.UserService } We have eliminated the need for extra types. The code under test is fully covered and we have made writing tests easier with dedicated test helper packages.
70. Adding Interfaces Too Soon The Right Time Don’t Start With Interfaces • Follow the convention: accept interfaces, return concrete types. • Begin with a concrete type. Only when you truly need multiple interchangeable types, start to consider introducing interfaces.. • Litmus Test: If you can write it without, you probably don’t need an interface.
71. Adding Interfaces Too Soon The Right Time Don’t Start With Interfaces • Follow the convention: accept interfaces, return concrete types. • Begin with a concrete type. Only when you truly need multiple interchangeable types, start to consider introducing interfaces.. • Litmus Test: If you can write it without, you probably don’t need an interface. Don’t Create Interfaces Solely for Testing • Don’t introduce escape-hatches to make production code more testable. • Prefer testing with real implementations (e.g., grpctest, thriftest, miniredis, etc). • Some dependencies, like Postgres, Kafka or BigQuery, don’t have a great alternative. Better an interface than no tests at all.
72. 03 Mutexes Before Channels
73. Mutexes Before Channels Closing a Closed Channel ch := make(chan int) close(ch) close(ch)
74. panic: close of closed channel
75. Mutexes Before Channels Sending on a Closed Channel ch := make(chan int) close(ch) ch <- 3
76. panic: send on closed channel
77. Mutexes Before Channels Sending without Receiver ch := make(chan int) ch <- 3 close(ch)
78. fatal error: all goroutines are asleep - deadlock!
79. Mutexes Before Channels Ranging over a Channel That’s Never Closed ch := make(chan int) go func() { ch <- 1 }() for v := range ch { fmt.Println(v) }
80. fatal error: all goroutines are asleep - deadlock!
81. ch := make(chan int) errors := make(chan error) done := make(chan struct{}) var wg sync.WaitGroup for _, v := range input { wg.Go(func() { resp, err := process(ctx, v) if err != nil { errors <- err } ch <- resp }) } go func() { wg.Wait() close(done) }() var resps []int for { select { case resp := <-ch: resps = append(resps, resp) case err := <-errors: return 0, err case <-done: return merge(resps...), nil case <-ctx.Done(): return 0, ctx.Err() } }
82. ch := make(chan int) errors := make(chan error) done := make(chan struct{}) var wg sync.WaitGroup for _, v := range input { wg.Go(func() { resp, err := process(ctx, v) if err != nil { errors <- err } ch <- resp }) } go func() { wg.Wait() close(done) }() var resps []int for { select { case resp := <-ch: resps = append(resps, resp) case err := <-errors: return 0, err case <-done: return merge(resps...), nil case <-ctx.Done(): return 0, ctx.Err() } }
83. ch := make(chan int) errors := make(chan error) done := make(chan struct{}) var wg sync.WaitGroup for _, v := range input { wg.Go(func() { resp, err := process(ctx, v) if err != nil { errors <- err } ch <- resp }) } go func() { wg.Wait() close(done) }() var resps []int for { select { case resp := <-ch: resps = append(resps, resp) case err := <-errors: return 0, err case <-done: return merge(resps...), nil case <-ctx.Done(): return 0, ctx.Err() } }
84. ch := make(chan int) errors := make(chan error) done := make(chan struct{}) g, ctx := errgroup.WithContext(ctx) for _, v := range input { g.Go(func() error { resp, err := process(ctx, v) if err != nil { errors <- err } ch <- resp }) } if err := g.Wait(); err != nil { return 0, err } var resps []int for { select { case resp := <-ch: resps = append(resps, resp) case err := <-errors: return 0, err case <-done: return merge(resps...), nil case <-ctx.Done(): return 0, ctx.Err() } }
85. ch := make(chan int) errors := make(chan error) g, ctx := errgroup.WithContext(ctx) for _, v := range input { g.Go(func() error { resp, err := process(ctx, v) if err != nil { errors <- err } ch <- resp }) } if err := g.Wait(); err != nil { return 0, err } var resps []int for { select { case resp := <-ch: resps = append(resps, resp) case err := <-errors: return 0, err } }
86. ch := make(chan int) g, ctx := errgroup.WithContext(ctx) for _, v := range input { g.Go(func() error { resp, err := process(ctx, v) if err != nil { return err } ch <- resp }) } if err := g.Wait(); err != nil { return 0, err } var resps []int for { select { case resp := <-ch: resps = append(resps, resp) } }
87. ch := make(chan int) g, ctx := errgroup.WithContext(ctx) for _, v := range input { g.Go(func() error { resp, err := process(ctx, v) if err != nil { return err } ch <- resp }) } if err := g.Wait(); err != nil { return 0, err } var resps []int for resp := <-ch: resps = append(resps, resp) }
88. var resps []int g, ctx := errgroup.WithContext(ctx) for _, v := range input { g.Go(func() error { resp, err := process(ctx, v) if err != nil { return err } ch <- resp }) } if err := g.Wait(); err != nil { return 0, err } for resp := <-ch: resps = append(resps, resp) }
89. var mu sync.Mutex var resps []int g, ctx := errgroup.WithContext(ctx) for _, v := range input { g.Go(func() error { resp, err := process(ctx, v) if err != nil { return err } mu.Lock() resps = append(resps, resp) mu.Unlock() }) } if err := g.Wait(); err != nil { return 0, err }
90. var mu sync.Mutex resps := make([]int, 0) g, ctx := errgroup.WithContext(ctx) for _, v := range input { g.Go(func() error { resp, err := process(ctx, v) if err != nil { return err } mu.Lock() resps = append(resps, resp) mu.Unlock() return nil }) } if err := g.Wait(); err != nil { return 0, err } return merge(resps...), nil
91. resps := make([]int, len(input)) g, ctx := errgroup.WithContext(ctx) for i, v := range input { g.Go(func() error { resp, err := process(ctx, v) if err != nil { return err } resps[i] = resp return nil }) } if err := g.Wait(); err != nil { return 0, err } return merge(resps...), nil
92. resps := make([]int, len(input)) g, ctx := errgroup.WithContext(ctx) for i, v := range input { g.Go(func() error { resp, err := process(ctx, v) if err != nil { return err } resps[i] = resp return nil }) } if err := g.Wait(); err != nil { return 0, err } return merge(resps...), nil
93. resps := make([]int, len(input)) g, ctx := errgroup.WithContext(ctx) for i, v := range input { g.Go(func() error { resp, err := process(ctx, v) if err != nil { return err } resps[i] = resp return nil }) } if err := g.Wait(); err != nil { return 0, err } return merge(resps...), nil
94. Mutexes Before Channels Start Simple, Advance One Step At a Time Channels are clever. In production, simpler is safer. • Begin with synchronous code. • Only add goroutines when profiling shows a bottleneck. • Use sync.Mutex and sync.WaitGroup for shared state. • Use go test -race to find data races. • Cannels shine for complex orchestration, not basic synchronization.
95. 04 Declare Close to Usage
96. Declare Close to Usage A Universal Pattern • Declare identifier (constants, variables, functions, types, etc.) in the file that needs them. • Export identifier only when they are needed outside of the package. • Within a function, declare variables as close as possible to where they end up being consumed.
97. Declare Close to Usage Limit Assignment Scope err := json.Unmarshal(b, &v) if err != nil { return nil, err }
98. Declare Close to Usage Limit Assignment Scope if err := json.Unmarshal(b, &v); err != nil { return nil, err }
99. Declare Close to Usage Limit Assignment Scope if err := json.Unmarshal(b, &v); err != nil { return nil, err }
100. Declare Close to Usage Limit Assignment Scope if err := json.Unmarshal(b, &v); err != nil { return nil, err } if err := v.Validate(); err != nil { return nil, err }
101. Declare Close to Usage Limit Assignment Scope err := json.Unmarshal(b, &v) if err != nil { return nil, err } err := v.Validate() if err != nil { return nil, err }
102. Declare Close to Usage Limit Assignment Scope err := json.Unmarshal(b, &v) if err != nil { return nil, err } err := v.Validate() if err != nil { return nil, err } no new variables on left side of :=
103. Declare Close to Usage Limit Assignment Scope err := json.Unmarshal(b, &v) if err != nil { return nil, err } err = v.Validate() if err != nil { return nil, err }
104. func fetch(auth auth, client Client, queries []string) ([]string, error) { var results []string var err error var authErr error if auth != nil { authErr = auth(func() error { results, err = client.PostSearch(queries) return err }) if authErr != nil { return nil, err } } else { results, err = client.PostSearch(queries) if err != nil { return nil, err } } return results, nil }
105. func fetch(auth auth, client Client, queries []string) ([]string, error) { var results []string var err error var authErr error if auth != nil { authErr = auth(func() error { results, err = client.PostSearch(queries) return err }) if authErr != nil { return nil, err } } else { results, err = client.PostSearch(queries) if err != nil { return nil, err } } return results, nil } Variable declaration, assignment, and use are spread out. Is that necessary?
106. func fetch(auth auth, client Client, queries []string) ([]string, error) { var results []string var err error if auth != nil { var authErr error authErr = auth(func() error { results, err = client.PostSearch(queries) return err }) if authErr != nil { return nil, err } } else { results, err = client.PostSearch(queries) if err != nil { return nil, err } } return results, nil }
107. func fetch(auth auth, client Client, queries []string) ([]string, error) { var results []string var err error if auth != nil { authErr := auth(func() error { results, err = client.PostSearch(queries) return err }) if authErr != nil { return nil, err } } else { results, err = client.PostSearch(queries) if err != nil { return nil, err } } return results, nil }
108. func fetch(auth auth, client Client, queries []string) ([]string, error) { var results []string var err error if auth != nil { authErr := auth(func() error { results, err = client.PostSearch(queries) return err }) if authErr != nil { return nil, err } } else { results, err = client.PostSearch(queries) if err != nil { return nil, err } } return results, nil } We check one error but return the other.
109. func fetch(auth auth, client Client, queries []string) ([]string, error) { var results []string var err error if auth != nil { err := auth(func() error { results, err = client.PostSearch(queries) return err }) if err != nil { return nil, err } } else { results, err = client.PostSearch(queries) if err != nil { return nil, err } } return results, nil }
110. func fetch(auth auth, client Client, queries []string) ([]string, error) { var results []string var err error if auth != nil { err := auth(func() error { results, err = client.PostSearch(queries) return err }) if err != nil { return nil, err } return results, nil } else { results, err = client.PostSearch(queries) if err != nil { return nil, err } } return results, nil }
111. func fetch(auth auth, client Client, queries []string) ([]string, error) { var results []string var err error if auth != nil { err := auth(func() error { results, err = client.PostSearch(queries) return err }) if err != nil { return nil, err } return results, nil } results, err := client.PostSearch(queries) if err != nil { return nil, err } return results, nil }
112. func fetch(auth auth, client Client, queries []string) ([]string, error) { if auth != nil { var results []string var err error err := auth(func() error { results, err = client.PostSearch(queries) return err }) if err != nil { return nil, err } return results, nil } results, err := client.PostSearch(queries) if err != nil { return nil, err } return results, nil }
113. func fetch(auth auth, client Client, queries []string) ([]string, error) { if auth != nil { var results []string var err error err := auth(func() error { results, err = client.PostSearch(queries) return err }) if err != nil { return nil, err } return results, nil } return client.PostSearch(queries) }
114. func fetch(auth auth, client Client, queries []string) ([]string, error) { if auth != nil { var results []string err := auth(func() (err error) { results, err = client.PostSearch(queries) return err }) if err != nil { return nil, err } return results, nil } return client.PostSearch(queries) }
115. Declare Close to Usage Don’t Let Your Ingredients Dry Out Keep Related Code Close Together • Don’t scatter identifiers. Declare constants, variables, and types where they’re used. • Two files need the same identifier? Keep it with the first one — don’t orphan it in a generic third file. • Export only once it’s needed. From Packages to Functions • Works at every level: packages, functions, and blocks. • Declare variables near their use to keep scope small. • Smaller scope reduces subtle bugs like shadowing. • Compact code groups make refactoring easier — easier to lift into helpers when everything it needs is already nearby.
116. 05 Avoid Runtime Panics
117. 05 Avoid Runtime Panics
118. Runtime Panics Check Your Inputs func selectNotifications(req *pb.Request) { max := req.Options.MaxNotifications req.Notifications = req.Notifications[:max] }
119. panic: runtime error: invalid memory address or nil pointer dereference [signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x4a4a4a]
120. Runtime Panics Check Your Inputs func selectNotifications(req *pb.Request) { max := req.Options.MaxNotifications req.Notifications = req.Notifications[:max] }
121. Runtime Panics Check Your Inputs func selectNotifications(req *pb.Request) { max := req.Options.MaxNotifications if len(req.Notifications) > max { req.Notifications = req.Notifications[:max] } }
122. Runtime Panics Check Your Inputs func selectNotifications(req *pb.Request) { if req == nil { return } max := req.Options.MaxNotifications if len(req.Notifications) > max { req.Notifications = req.Notifications[:max] } }
123. Runtime Panics Check Your Inputs When to Check • If data comes from outside (requests, external stores), validate it first. • Protect yourself from runtime panics on inputs you don’t control. How Not to Overdo It • Don’t litter the code with endless if x == nil checks. • Rule of thumb: if you control the flow, trust Go’s error handling. • Remember: error handling is your contract — don’t duplicate it with nil checks. • Balance safety with readability: handle real risks, keep the happy path clean.
124. Runtime Panics Check Nil Before Dereferencing type FeedItem struct { Score *float64 `json:"score"` } func sumFeedScores(feed *Feed) error { var scores float64 for _, item := range feed.Items { scores += *item.Score } return nil }
125. Runtime Panics Check Nil Before Dereferencing type FeedItem struct { Score *float64 `json:"score"` } func sumFeedScores(feed *Feed) error { var scores float64 for _, item := range feed.Items { if item.Score == nil { continue } scores += *item.Score } return nil }
126. Runtime Panics Design for Pointer Safety Best pointer safety: Eliminate the need to explicitly dereference. type FeedItem struct { Score float64 `json:"score"` } func sumFeedScores(feed *Feed) error { var scores float64 for _, item := range feed.Items { scores += item.Score } return nil }
127. Runtime Panics Design for Pointer Safety func main() { db, err := dep.NewVectorDB() if err != nil { log.Fatal(err) } job := cron.NewJob("indexer", *db) if err := job.Run(); err != nil { log.Fatal(err) } }
128. Runtime Panics Design for Pointer Safety func main() { db, err := dep.NewVectorDB() if err != nil { log.Fatal(err) } job := cron.NewJob("indexer", db) if err := job.Run(); err != nil { log.Fatal(err) } }
129. 06 Minimize Indentatio n
130. func fetch(auth auth, client Client, queries []string) ([]string, error) { var results []string var err error var authErr error if auth != nil { authErr = auth(func() error { results, err = client.PostSearch(queries) return err }) if authErr != nil { return nil, err } } else { results, err = client.PostSearch(queries) if err != nil { return nil, err } } return results, nil }
131. func fetch(auth auth, client Client, queries []string) ([]string, error) { if auth != nil { var results []string err := auth(func() (err error) { results, err = client.PostSearch(queries) return err }) if err != nil { return nil, err } return results, nil } return client.PostSearch(queries) }
132. Minimize Indentation BAD: Wraps All Logic Inside if err := doSomething(); err == nil { if ok := check(); ok { process() } else { return errors.New("check failed") } } else { return err }
133. Minimize Indentation Good: Return Early, Flatter Structure if err := doSomething(); err != nil { return err } if !check() { return errors.New("check failed") } process()
134. func getQueriesByID(items []*Item) map[string][]string { queriesByID := make(map[string][]string) for _, item := range items { var queryCount int if item.Queries != nil { queries := make([]string, 0) for _, query := range item.Queries.Values { if query != "" { queries = append(queries, query) queryCount++ } } queriesByID[item.ID] = queries observeQueryCount(item.ID, queryCount) } } return queriesByID }
135. func getQueriesByID(items []*Item) map[string][]string { queriesByID := make(map[string][]string) for _, item := range items { var queryCount int if item.Queries == nil { continue } queries := make([]string, 0) for _, query := range item.Queries.Values { if query != "" { queries = append(queries, query) queryCount++ } } queriesByID[item.ID] = queries observeQueryCount(item.ID, queryCount) } return queriesByID }
136. func getQueriesByID(items []*Item) map[string][]string { queriesByID := make(map[string][]string) for _, item := range items { if item.Queries == nil { continue } var queryCount int queries := make([]string, 0) for _, query := range item.Queries.Values { if query != "" { queries = append(queries, query) queryCount++ } } queriesByID[item.ID] = queries observeQueryCount(item.ID, queryCount) } return queriesByID }
137. func getQueriesByID(items []*Item) map[string][]string { queriesByID := make(map[string][]string) for _, item := range items { if item.Queries == nil { continue } var queryCount int queries := make([]string, 0) for _, query := range item.Queries.Values { if query == "" { continue } queries = append(queries, query) queryCount++ } queriesByID[item.ID] = queries observeQueryCount(item.ID, queryCount) } return queriesByID }
138. func getQueriesByID(items []*Item) map[string][]string { queriesByID := make(map[string][]string) for _, item := range items { var queryCount int if item.Queries != nil { queries := make([]string, 0) for _, query := range item.Queries.Values { if query != "" { queries = append(queries, query) queryCount++ } } queriesByID[item.ID] = queries observeQueryCount(item.ID, queryCount) } } return queriesByID }
139. func getQueriesByID(items []*Item) map[string][]string { queriesByID := make(map[string][]string) for _, item := range items { if item.Queries == nil { continue } var queryCount int queries := make([]string, 0) for _, query := range item.Queries.Values { if query == "" { continue } queries = append(queries, query) queryCount++ } queriesByID[item.ID] = queries observeQueryCount(item.ID, queryCount) } return queriesByID }
140. func getQueriesByID(items []*Item) map[string][]string { queriesByID := make(map[string][]string) for _, item := range items { if item.Queries == nil { continue } queries := getQueries(item) queriesByID[item.ID] = queries observeQueryCount(item.ID, len(queries)) } return queriesByID } func getQueries(item *Item) []string { queries := make([]string, 0) for _, query := range skipEmpty(item.Queries.Values) { queries = append(queries, query) } return queries }
141. func getQueriesByID(items []*Item) map[string][]string { queriesByID := make(map[string][]string) for _, item := range items { if item.Queries == nil { continue } queries := slices.Collect(skipEmpty(item.Queries.Values)) queriesByID[item.ID] = queries observeQueryCount(item.ID, len(queries)) } return queriesByID }
142. 07 Avoid Catch-All Packages and Files
143. util.go misc.go constants.go
144. util.go misc.go constants.go interfaces/interfaces.go
145. “ Sweatpants are a sign of defeat. You lost control of your life, so you bought some sweatpants. Karl Lagerfeld
146. “ Util packages are a sign of defeat. You lost control of your code base, so you created some util packages. Gnarl Largerfur
147. “ Util packages are a sign of defeat. You lost control of your code base, so you created some util packages. NOT ACTUAL CODE REVIEW COMMENT Gnarl Largerfur
148. Avoid Catch-All Packages and Files Prefer Locality over Hierarchy • Code is easier to understand when it’s near what it affects. • Abstract organization might feel tidy, but it often hides purpose. • Be specific: name after domain or functionality. • Group by meaning, not by type.
149. 08 Order Declarations by Importance
150. func Trim(s, cutset string) string { // ... return trimLeftUnicode(trimRightUnicode(s, cutset), cutset) } func trimLeftByte(s string, c byte) string { ... } func trimRightUnicode(s, cutset string) string { ... } Order Declarations by Importance Most Important Code to the Top
151. func trimLeftByte(s string, c byte) string { ... } func trimRightUnicode(s, cutset string) string { ... } func Trim(s, cutset string) string { // ... return trimLeftUnicode(trimRightUnicode(s, cutset), cutset) } Order Declarations by Importance Most Important Code to the Top
152. func Trim(s, cutset string) string { // ... return trimLeftUnicode(trimRightUnicode(s, cutset), cutset) } func trimLeftByte(s string, c byte) string { ... } func trimRightUnicode(s, cutset string) string { ... } Order Declarations by Importance Most Important Code to the Top
153. In Go, functions don’t need to be declared before use (no forward declarations). Declaration order still matters for readability. • Put exported, API-facing functions first. • Follow with helper functions, which are implementation details. Order functions by importance, not by dependency. This way, readers see the most important entry points up front. Order Declarations by Importance Most Important Code to the Top
154. Order Declarations by Importance Most Important Code to the Top type mockBigQuery struct{ orders: []model.Order } func (m *mockBigQuery) ListOrders() ([]model.Order, error) { return m.orders, nil } func TestCreateOrder(t *testing.T) { bq := &mockBigQuery{} // ... }
155. Order Declarations by Importance Most Important Code to the Top func TestCreateOrder(t *testing.T) { bq := &mockBigQuery{} // ... } type mockBigQuery struct{ orders: []model.Order } func (m *mockBigQuery) ListOrders() ([]model.Order, error) { return m.orders, nil }
156. 09 Name Well
157. Name Well Avoid Type Suffix userMap map[string]*User idStr string injectFn func()
158. Variable names should describe contents, not type. Adding type info makes code less clear and no more type safe: Name Well Avoid Type Suffix userMap map[string]*User idStr string injectFn func()
159. Variable names should describe contents, not type. Adding type info makes code less clear and no more type safe: Name Well Avoid Type Suffix userMap map[string]*User idStr string injectFn func() ❌ BAD
160. Variable names should describe contents, not type. Adding type info makes code less clear and no more type safe: Name Well Avoid Type Suffix userMap map[string]*User idStr string injectFn func() userByID map[string]*User id string inject func() ❌ BAD ✅ Good
161. It is not uncommon to see the use of one character variables in Go. While this conflicts with Clear Naming, it can make sense in some cases. Let the following metric guide you: the bigger the scope of declaring a variable and using it, the less likely it should have a very short or cryptic name. Name Well Variable Length
162. Think about how code reads at the call site: Name Well Packages and Exported Identifiers
163. Think about how code reads at the call site: Name Well Packages and Exported Identifiers db := test.NewDatabaseFromFile(...) _, err := f.Seek(0, common.SeekStart) b := helper.Marshal(curve, x, y) c := consumer.NewConsumerHandler(...)
164. Think about how code reads at the call site: Name Well Packages and Exported Identifiers db := spannertest.NewDatabaseFromFile(...) _, err := f.Seek(0, io.SeekStart) b := elliptic.Marshal(curve, x, y) c := consumer.NewHandler(...)
165. 10 Document the Why, Not the What
166. func EscapeDoubleQuotes(s string) string { if strings.HasPrefix(s, `"`) && strings.HasSuffix(s, `"`) { core := strings.TrimPrefix(strings.TrimSuffix(s, `"`), `"`) escaped := strings.ReplaceAll(core, `"`, `\"`) escaped = strings.ReplaceAll(escaped, `\\"`, `\"`) return fmt.Sprintf(`"%s"`, escaped) } return s } Document the Why, Not the What Justify the Code’s Existence
167. // Escapes internal double quotes by replacing `"` with `\"`. func EscapeDoubleQuotes(s string) string { if strings.HasPrefix(s, `"`) && strings.HasSuffix(s, `"`) { core := strings.TrimPrefix(strings.TrimSuffix(s, `"`), `"`) escaped := strings.ReplaceAll(core, `"`, `\"`) escaped = strings.ReplaceAll(escaped, `\\"`, `\"`) return fmt.Sprintf(`"%s"`, escaped) } return s } Document the Why, Not the What Justify the Code’s Existence
168. // We can sometimes receive a label like: ""How-To"" because the frontend // wraps user-provided labels in quotes, even when the value itself // contains literal `"` characters. In this case, attempt to escape all // internal double quotes, leaving only the outermost ones unescaped. func EscapeDoubleQuotes(s string) string { if strings.HasPrefix(s, `"`) && strings.HasSuffix(s, `"`) { core := strings.TrimPrefix(strings.TrimSuffix(s, `"`), `"`) escaped := strings.ReplaceAll(core, `"`, `\"`) escaped = strings.ReplaceAll(escaped, `\\"`, `\"`) return fmt.Sprintf(`"%s"`, escaped) } return s } Document the Why, Not the What Justify the Code’s Existence
169. // Escapes internal double quotes by replacing `"` with `\"`. func EscapeDoubleQuotes(s string) string { if strings.HasPrefix(s, `"`) && strings.HasSuffix(s, `"`) { core := strings.TrimPrefix(strings.TrimSuffix(s, `"`), `"`) escaped := strings.ReplaceAll(core, `"`, `\"`) escaped = strings.ReplaceAll(escaped, `\\"`, `\"`) return fmt.Sprintf(`"%s"`, escaped) } return s } Document the Why, Not the What Justify the Code’s Existence
170. Document the Why, Not the What Justify the Code’s Existence Write for the future reader (that’s you too). Explain the why! When writing comments, your goal is to communicate purpose, not just restate the code. A meaningful description should answer why the change is needed and how you are solving it. • Pull request description: explain why this change matters. • Code comments: document intent, not mechanics. • Future readers should be able to understand the motivation behind your choices. Readers can usually see what the code does, but often struggle to understand why it was written in the first place.
171. Summar y Su y Summary Summar y Summar y Summar y ar Summar y S y r Summar y Summar y
172. Writing Better Go Most “style” comments aren’t about aesthetics — they’re about avoiding real production pain. Patterns repeat: what looks like nitpicking in one pull request often shows up later as a bug, an outage, or unreadable code. The real goal isn’t perfection, it’s reducing friction: for readers, maintainers, and your future self. Every rule of thumb (error handling, interfaces, mutexes, naming) is really about the same thing: make the code obvious, safe, and easy to move forward. Code review isn’t just about shipping features safely. It’s where we teach, learn, and build shared intuition together. The Bigger Picture
173. Thank you
174. Questions ?