I spent 4 months refactoring a Go CLI and tracked every change in a catalog. When I looked at the 60 entries, the largest cluster wasn't about types or error handling — it was about where code lives.
Coupling and cohesion problems don't cause bugs. They cause something worse: developers who can't find anything. A function in the wrong file. A type in a junk drawer package. A feature spread across 3 files when it should be in 1.
Here are 8 patterns I fixed, each with before/after code from the actual commits.
Pattern 1: The Junk Drawer File
The Problem
Five types.go files contained 125 types with no organizational principle:
setup/types.go — 40 types (doctor, config, status, init, env, alias, context)
evidence/types.go — 28 types (enums, params, snapshots, bundles, providers)
dto/types.go — 23 types (findings, results, remediations)
reporting/types.go — 22 types (baseline, CI diff, reports, enforce, prompts)
usecase/types.go — 12 types (gate, fix, trace, apply, verify)
When you grepped for "Gate," you found it in usecase/types.go alongside trace, apply, and verify types. The file was organized by technical role (all types together) instead of business concern (gate types with gate logic).
The Fix
Split each file by business concern:
setup/types.go (40 types) → 8 files:
doctor_types.go
config_types.go
status_types.go
generate_types.go
init_types.go
env_types.go
alias_types.go
context_types.go
usecase/types.go (12 types) → 6 files:
gate.go
fix.go
trace.go
apply.go
verify.go
fixloop.go
Now grepping for "Gate" shows only gate-related types. Each file represents one business concern. The file name tells you what's inside without opening it.
Files changed: 5 files split into 30+ focused files.
Pattern 2: The Anemic File
The Problem
The opposite of junk drawers: 25 files with one function each.
baseline/run.go — empty stub (0 functions)
cidiff/run.go — empty stub (0 functions)
kernel/normalize.go — 1 function (EnsureTrailingSlash)
evaluation/scoring.go — 1 helper
sanitize/common.go — 1 type
Opening kernel/normalize.go to find a single 4-line function meant more time navigating than reading. These files existed because "one file per function" was applied too aggressively.
The Fix
Merge into the files that own the domain concept:
kernel/normalize.go → merged into kernel/identity.go
evaluation/scoring.go → merged into evaluation/confidence.go
sanitize/common.go → merged into sanitize/sanitize.go
baseline/run.go → deleted (empty)
Rule: If a file has ≤1 function and ≤30 lines, it should merge into its logical neighbor. Each domain concept lives in one file, not two.
Files changed: 25 anemic files merged, 45 total files touched.
Pattern 3: The Utility Junk Drawer
The Problem
A single logic.go file mixed four unrelated concerns in the S3 policy analyzer:
// logic.go (280 lines)
func classifyAction(action string) ActionCategory { ... } // Action analysis
func extractPrincipalAccount(arn string) string { ... } // Principal parsing
func resolveConditionScope(ca ConditionAnalysis) Scope { ... } // Network scoping
func normalizeStringOrArray(v any) []string { ... } // JSON normalization
A developer adding a new S3 action had to scroll past principal extraction, network scoping, and JSON normalization to find the action classifier.
The Fix
Split by domain concern:
logic.go (280 lines) → 4 files:
actions.go — IAM action → security category mapping
principals.go — AWS principal ARN/account ID extraction
scoping.go — condition analysis → network scope resolution
normalize.go — AWS JSON string-or-array helper
A developer adding a new S3 action goes to actions.go. No scrolling past unrelated code. The file name is the search result.
Pattern 4: The Boilerplate Clone
The Problem
14 S3 compliance controls each had identical loop+filter+parse boilerplate:
// BEFORE: Every control had this identical preamble
func (ctl *accessBlockPublic) Evaluate(snap asset.Snapshot) Result {
for _, a := range snap.Assets {
if !isS3Bucket(a) {
continue
}
props := ParseS3Properties(a)
// ... actual control logic here (the only unique part) ...
}
return ctl.PassResult()
}
The loop, filter, and parse were copy-pasted across all 14 files. Adding a new control meant copying 8 lines of infrastructure before writing 1 line of business logic.
The Fix
Extract the infrastructure into a callback pattern:
// AFTER: Control implements only the unique logic
func (ctl *accessBlockPublic) Evaluate(snap asset.Snapshot) Result {
return ctl.evaluateS3Buckets(snap, func(a asset.Asset, props S3Properties) *Result {
if props.Controls.IsPublicAccessFullyBlocked() {
return nil // safe — skip
}
r := ctl.FailResult("BPA not fully enabled", "Enable all four flags")
return &r
})
}
The helper:
func (d *Definition) evaluateS3Buckets(snap asset.Snapshot, check func(asset.Asset, S3Properties) *Result) Result {
for _, a := range snap.Assets {
if !isS3Bucket(a) {
continue
}
props := ParseS3Properties(a)
if result := check(a, props); result != nil {
return *result
}
}
return d.PassResult()
}
14 copies of the loop → 1 helper. Net -41 lines. New controls implement a single callback function with zero boilerplate.
Pattern 5: The Fat Struct
The Problem
A summary struct mixed three unrelated concerns:
// BEFORE: 10 fields mixing counts, gating, and metadata
type Summary struct {
TotalControls int // ← counting
PassCount int // ← counting
FailCount int // ← counting
WarnCount int // ← counting
FailOn Severity // ← gating
Gated bool // ← gating
GatedFindingCount int // ← gating
VulnSourceUsed string // ← metadata
EvidenceFreshness time.Duration // ← metadata
StaveVersion string // ← metadata
}
RecomputeSummary() needed to reset the counts but preserve the metadata. One caller forgot to preserve StaveVersion after a recompute.
The Fix
Split by concern:
// AFTER: Three cohesive structs
type ResultCounts struct {
Total int
Pass int
Fail int
Warn int
}
type GatingInfo struct {
FailOn Severity
Gated bool
GatedFindingCount int
}
type AuditMeta struct {
VulnSourceUsed string
EvidenceFreshness time.Duration
StaveVersion string
}
RecomputeSummary() now takes ResultCounts and returns ResultCounts. It physically cannot touch gating or metadata because they're in different structs.
Files changed: 18 — the DTO layer maps from the nested structs to the same flat JSON output.
Pattern 6: The Mutable Global
The Problem
Confidence thresholds were set via a package-level global with a setter:
// BEFORE: Mutable global state
var confidenceThresholds = struct {
high int
med int
}{high: 4, med: 2}
func SetConfidenceThresholds(high, med int) {
confidenceThresholds.high = high
confidenceThresholds.med = med
}
This was called once at startup but the global was accessible from any goroutine. Not thread-safe. Not testable (tests leak state between runs). Not traceable (who set it? when?).
The Fix
Replace with a struct threaded through the dependency chain:
// AFTER: Explicit dependency — no global state
type ConfidenceCalculator struct {
HighMultiplier int
MedMultiplier int
}
func DefaultConfidenceCalculator() ConfidenceCalculator {
return ConfidenceCalculator{HighMultiplier: 4, MedMultiplier: 2}
}
// Threaded through Runner → strategyDeps → strategies
type Runner struct {
Confidence ConfidenceCalculator
// ...
}
Configuration flows through the constructor, not through a global setter. Thread-safe by design. Testable without cleanup.
Pattern 7: The Misplaced Function
The Problem
Two functions in core/evaluation were only called by adapters/output/text:
// BEFORE: In core/evaluation (called only by one adapter)
func GroupViolationsByDomain(findings []Finding) map[string][]Finding { ... }
func DomainCount(findings []Finding) int { ... }
These are presentation helpers — they group findings for text output. They don't belong in the core evaluation package.
The Fix
Move to the only consumer:
// AFTER: In adapters/output/text (the sole caller)
func groupViolationsByDomain(findings []evaluation.Finding) map[string][]evaluation.Finding { ... }
func domainCount(findings []evaluation.Finding) int { ... }
The functions became unexported (lowercase) because they're internal to the adapter. Core evaluation no longer carries presentation logic.
Similarly, Sanitized() methods on core domain types were moved to the app layer:
// BEFORE: Sanitization method on core domain type
func (m Misconfiguration) Sanitized(san Sanitizer) Misconfiguration { ... }
// AFTER: Sanitization logic in app layer (enrich.go)
func sanitizeFinding(san Sanitizer, f Finding) Finding { ... }
Core types are now pure data. Sanitization is a presentation concern that lives in the app layer.
Pattern 8: The Duplicate Package
The Problem
Two packages served the same purpose:
internal/sanitize/ — Sanitizer, Policy, constants
internal/sanitize/scrub/ — Engine with Snapshot() and Map() methods
The scrub sub-package was created to avoid a "large file." But its only consumer was sanitize itself. Two packages for one concept.
The Fix
Merge into one:
// BEFORE: Two packages
import "internal/sanitize"
import "internal/sanitize/scrub"
engine := scrub.NewEngine(sanitize.NewSanitizer(policy))
engine.Snapshot(snap)
// AFTER: One package
import "internal/sanitize"
san := sanitize.New(policy)
san.Snapshot(snap)
scrub/ was deleted entirely. One package, one import, one concept.
The Metrics
| Metric | Before | After |
|---|---|---|
| Junk drawer files | 5 (125 types mixed) | 0 (each file = 1 concern) |
| Anemic files | 25 (≤1 function each) | 0 (merged into neighbors) |
| Boilerplate copies | 14 (S3 control loop) | 1 (shared helper) |
| Mutable globals | 1 (confidence thresholds) | 0 (threaded dependency) |
| Misplaced functions | 4 (core hosting presentation) | 0 (moved to adapters) |
| Duplicate packages | 1 (sanitize + scrub) | 0 (merged) |
| Discoverability score | 7/10 | 9/10 |
The Principle
Coupling is about how much changes cascade. Cohesion is about how easily you find things.
High cohesion means: when you need to change "gating logic," you open one file in one package. Low coupling means: when you change "gating logic," nothing in "trace" or "apply" needs to change.
Both are served by one question: does this code change for the same reason as its neighbors?
- If yes → keep together (merge anemic files, combine duplicate packages)
- If no → separate (split junk drawers, extract boilerplate, move misplaced functions)
The metric that matters isn't lines of code or file count. It's: how many files do you open to understand one feature? After these 8 fixes, the answer dropped from 4-5 to 1-2.
*These 8 patterns were identified across 60 refactorings in Stave, an offline configuration safety evaluator.
Top comments (0)