DEV Community

Bala Paranj
Bala Paranj

Posted on

8 Coupling and Cohesion Fixes That Made a Go CLI Navigable

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)
Enter fullscreen mode Exit fullscreen mode

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
Enter fullscreen mode Exit fullscreen mode

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
Enter fullscreen mode Exit fullscreen mode

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)
Enter fullscreen mode Exit fullscreen mode

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
Enter fullscreen mode Exit fullscreen mode

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
Enter fullscreen mode Exit fullscreen mode

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()
}
Enter fullscreen mode Exit fullscreen mode

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
    })
}
Enter fullscreen mode Exit fullscreen mode

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()
}
Enter fullscreen mode Exit fullscreen mode

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
}
Enter fullscreen mode Exit fullscreen mode

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
}
Enter fullscreen mode Exit fullscreen mode

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
}
Enter fullscreen mode Exit fullscreen mode

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
    // ...
}
Enter fullscreen mode Exit fullscreen mode

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 { ... }
Enter fullscreen mode Exit fullscreen mode

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 { ... }
Enter fullscreen mode Exit fullscreen mode

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 { ... }
Enter fullscreen mode Exit fullscreen mode

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
Enter fullscreen mode Exit fullscreen mode

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)
Enter fullscreen mode Exit fullscreen mode

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)