DEV Community

Bala Paranj
Bala Paranj

Posted on

Stop Indenting — 6 Patterns for Flattening Nested if/else in Go

#go #programming #refactoring

Every level of indentation is a level of mental context the reader must hold. Three levels deep and you're debugging the structure, not the logic. Four levels and you've lost the plot entirely.

I found 6 patterns of unnecessary nesting across a Go CLI and flattened each one. Here's the before/after for each.

Pattern 1: Inverted Loop Condition — Early Continue

Before: Entire loop body nested inside a conditional 

for _, sf := range suffixes {
    if strings.HasSuffix(upper, sf.label) {
        numStr := strings.TrimSpace(s[:len(s)-len(sf.label)])
        n, err := strconv.ParseInt(numStr, 10, 64)
        if err != nil {
            return 0, fmt.Errorf("invalid byte size %q: %w", s, err)
        }
        if n <= 0 {
            return 0, fmt.Errorf("byte size must be positive: %q", s)
        }
        return n * sf.multiplier, nil
    }
}
Enter fullscreen mode Exit fullscreen mode

The entire loop body — parsing, validation, return — is nested one level deep because of the HasSuffix check. The reader sees indentation and thinks "this is conditional." But it's actually the main logic — the continue case is the uninteresting one.

After: Invert and continue 

for _, sf := range suffixes {
    if !strings.HasSuffix(upper, sf.label) {
        continue
    }
    numStr := strings.TrimSpace(s[:len(s)-len(sf.label)])
    n, err := strconv.ParseInt(numStr, 10, 64)
    if err != nil {
        return 0, fmt.Errorf("invalid byte size %q: %w", s, err)
    }
    if n <= 0 {
        return 0, fmt.Errorf("byte size must be positive: %q", s)
    }
    return n * sf.multiplier, nil
}
Enter fullscreen mode Exit fullscreen mode

Same logic. One less indentation level. The guard clause says "skip non-matches" and the main logic lives at the top level of the loop. The reader doesn't need to track which brace closes which block.

Pattern 2: Extract to Flatten Triple-Nested Loops

Before: Three nested loops with error handling at depth 4 

func BuildTimelinesPerControl(
    controls []policy.ControlDefinition,
    snapshots []asset.Snapshot,
    celEval policy.PredicateEval,
) (map[kernel.ControlID]map[asset.ID]*asset.Timeline, error) {
    // ... init ...
    for _, snap := range snapshots {
        captureTime := snap.CapturedAt
        for _, a := range snap.Assets {
            for _, ctl := range controls {
                timelines := timelinesByControl[ctl.ID]
                t, exists := timelines[a.ID]
                if !exists {
                    var err error
                    t, err = asset.NewTimeline(a)
                    if err != nil {
                        return nil, fmt.Errorf("build timeline for control %s: %w", ctl.ID, err)
                    }
                    timelines[a.ID] = t
                }
                isUnsafe := checkUnsafe(ctl, a, snap, celEval)
                if err := t.RecordObservation(captureTime, isUnsafe); err != nil {
                    return nil, fmt.Errorf("record observation for control %s, asset %s: %w", ctl.ID, a.ID, err)
                }
                t.SetAsset(a)
            }
        }
    }
    return timelinesByControl, nil
}
Enter fullscreen mode Exit fullscreen mode

Four levels of indentation: for snapshots → for assets → for controls → if !exists. The error return at the deepest level is 5 levels from the function signature. Reading this requires mentally tracking three loop variables and a conditional simultaneously.

After: Extract the inner loop into a named function 

for _, snap := range snapshots {
    for _, a := range snap.Assets {
        if err := recordAssetObservation(a, snap, controls, celEval, timelinesByControl); err != nil {
            return nil, err
        }
    }
}
Enter fullscreen mode Exit fullscreen mode

The extracted function:

// recordAssetObservation evaluates a single asset against all controls at one
// point in time, updating the corresponding timelines.
func recordAssetObservation(
    a asset.Asset,
    snap asset.Snapshot,
    controls []policy.ControlDefinition,
    celEval policy.PredicateEval,
    timelinesByControl map[kernel.ControlID]map[asset.ID]*asset.Timeline,
) error {
    for _, ctl := range controls {
        timelines := timelinesByControl[ctl.ID]
        t, exists := timelines[a.ID]
        if !exists {
            var err error
            t, err = asset.NewTimeline(a)
            if err != nil {
                return fmt.Errorf("build timeline for control %s: %w", ctl.ID, err)
            }
            timelines[a.ID] = t
        }
        isUnsafe := checkUnsafe(ctl, a, snap, celEval)
        if err := t.RecordObservation(snap.CapturedAt, isUnsafe); err != nil {
            return fmt.Errorf("record observation for control %s, asset %s: %w", ctl.ID, a.ID, err)
        }
        t.SetAsset(a)
    }
    return nil
}
Enter fullscreen mode Exit fullscreen mode

The outer function dropped from 4 nesting levels to 2. The extracted function has the same logic but starts at a lower nesting baseline. The function name documents the boundary: "this is what happens for one asset at one point in time."

Pattern 3: Guard Clauses Instead of if/else Chains

Before: Variable declared, then set through if/else 

func NewObservationRepository(
    fileLoader ObservationRepository,
    stdinLoader ObservationRepository,
) (ObservationRepository, error) {
    var loader ObservationRepository
    if p.UseStdin {
        if stdinLoader == nil {
            return nil, fmt.Errorf("stdin observation repository is not configured")
        }
        loader = stdinLoader
    } else {
        if fileLoader == nil {
            return nil, fmt.Errorf("observation repository is not configured")
        }
        loader = fileLoader
    }

    if !p.UseStdin && p.ManifestPath != "" {
        if cfg, ok := loader.(integrityCheckConfigurer); ok {
            // ... configure integrity checks ...
        }
    }
    return loader, nil
}
Enter fullscreen mode Exit fullscreen mode

The loader variable is declared, then conditionally assigned through an if/else. The reader must track loader through two branches and a post-conditional feature gate. The !p.UseStdin && p.ManifestPath != "" check is redundant — it re-tests the condition that was already resolved.

After: Early return for each path 

func NewObservationRepository(
    fileLoader ObservationRepository,
    stdinLoader ObservationRepository,
) (ObservationRepository, error) {

    // 1. Select the Loader (Guard Clauses)
    if p.UseStdin {
        if stdinLoader == nil {
            return nil, fmt.Errorf("stdin observation repository is not configured")
        }
        return stdinLoader, nil // Happy path for Stdin ends here
    }

    // 2. Happy path for File-based Loading
    if fileLoader == nil {
        return nil, fmt.Errorf("observation repository is not configured")
    }

    // 3. Conditional Feature: Integrity Checks
    if p.ManifestPath != "" {
        if cfg, ok := fileLoader.(integrityCheckConfigurer); ok {
            // ... configure integrity checks ...
        }
    }
    return fileLoader, nil
}
Enter fullscreen mode Exit fullscreen mode

No else. No loader variable. Each path terminates with its own return. The stdin path exits early. The file path continues to the integrity check. The reader follows one linear path instead of tracking a shared variable across branches.

Pattern 4: Merge Duplicate Guards

Before: Two separate guard clauses with identical bodies 

_, found, err := projconfig.FindNearestFile(appconfig.ProjectConfigFile)
if err != nil {
    fmt.Fprintf(a.Root.ErrOrStderr(),
        "No Stave project found in this directory tree. Run `%s` to create one.\n",
        cliCommand("init"))
    return
}
if !found {
    fmt.Fprintf(a.Root.ErrOrStderr(),
        "No Stave project found in this directory tree. Run `%s` to create one.\n",
        cliCommand("init"))
}
Enter fullscreen mode Exit fullscreen mode

Two guard clauses. Identical Fprintf call. Identical message. The only difference: one has return, the other doesn't. But it's the last statement in the function, so the effect is the same.

After: Single merged condition 

_, found, err := projconfig.FindNearestFile(appconfig.ProjectConfigFile)
if err != nil || !found {
    fmt.Fprintf(a.Root.ErrOrStderr(),
        "No Stave project found in this directory tree. Run `%s` to create one.\n",
        cliCommand("init"))
}
Enter fullscreen mode Exit fullscreen mode

Same logic. Half the code. When two guards produce the same outcome, merge them.

A second merge in the same commit ordered the cheaper check first:

// BEFORE: expensive check first
if strings.Contains(message, "\nRun:") {
    return message
}
if len(args) == 0 {
    return message
}

// AFTER: cheap check first
if len(args) == 0 || strings.Contains(message, "\nRun:") {
    return message
}
Enter fullscreen mode Exit fullscreen mode

len(args) is O(1). strings.Contains scans the string. Put the cheap check first — if it short-circuits, the expensive check never runs.

Pattern 5: Split Monolith into Guarded Helpers

Before: 35 cognitive complexity in one function 

func PrepareEvaluationContext(req EvalContextRequest) (EvalContext, error) {
    var ec EvalContext

    if !req.SkipPathInference {
        resolver, err := projctx.NewResolver()
        if err != nil {
            return EvalContext{}, ui.WithHint(
                fmt.Errorf("resolve project context: %w", err),
                ui.ErrHintProjectContext,
            )
        }
        engine := projctx.NewInferenceEngine(resolver)
        ec.Resolver = resolver
        ec.Engine = engine
        ec.ControlsDir = fsutil.CleanUserPath(req.ControlsDir)
        ec.ObservationsDir = fsutil.CleanUserPath(req.ObservationsDir)

        if !req.ControlsChanged {
            if inferred := engine.InferDir("controls", ""); inferred != "" {
                ec.ControlsDir = inferred
            }
        }
        if !req.ObsChanged {
            if inferred := engine.InferDir("observations", ""); inferred != "" {
                ec.ObservationsDir = inferred
            }
        }
        // ... validation checks ...
    } else {
        ec.ControlsDir = fsutil.CleanUserPath(req.ControlsDir)
        ec.ObservationsDir = fsutil.CleanUserPath(req.ObservationsDir)
    }

    // ... 30 more lines of flag parsing ...
    return ec, nil
}
Enter fullscreen mode Exit fullscreen mode

Path inference, directory validation, duration parsing, clock resolution, and format resolution — all in one function. The if !req.SkipPathInference block contains the path logic and an else that duplicates two lines. Every concern adds to the cognitive complexity score.

After: Two focused helpers with early returns 

func PrepareEvaluationContext(req EvalContextRequest) (EvalContext, error) {
    var ec EvalContext
    if err := resolvePaths(&ec, req); err != nil {
        return EvalContext{}, err
    }
    if err := resolveFlags(&ec, req); err != nil {
        return EvalContext{}, err
    }
    return ec, nil
}
Enter fullscreen mode Exit fullscreen mode

The resolvePaths helper uses an inverted early return to eliminate the else:

func resolvePaths(ec *EvalContext, req EvalContextRequest) error {
    if req.SkipPathInference {
        ec.ControlsDir = fsutil.CleanUserPath(req.ControlsDir)
        ec.ObservationsDir = fsutil.CleanUserPath(req.ObservationsDir)
        return nil
    }

    // Full inference logic at top level — no else needed
    resolver, err := projctx.NewResolver()
    if err != nil {
        return ui.WithHint(
            fmt.Errorf("resolve project context: %w", err),
            ui.ErrHintProjectContext,
        )
    }
    // ... rest of inference at indentation level 1 ...
    return nil
}
Enter fullscreen mode Exit fullscreen mode

The if req.SkipPathInference check handles the simple case and returns. The complex inference logic lives at the top level of the function — no wrapping if needed.

Pattern 6: Deduplicate Identical Branches with Shared Helper

Before: Three switch cases with identical structure 

func walkNode(node parse.Node) error {
    switch n := node.(type) {
    case *parse.ActionNode:
        return walkPipe(n.Pipe)
    case *parse.RangeNode:
        if n.Pipe != nil {
            if err := walkPipe(n.Pipe); err != nil {
                return err
            }
        }
        if n.List != nil {
            if err := walkNodes(n.List.Nodes); err != nil {
                return err
            }
        }
        if n.ElseList != nil {
            if err := walkNodes(n.ElseList.Nodes); err != nil {
                return err
            }
        }
    case *parse.IfNode:
        if n.Pipe != nil {
            if err := walkPipe(n.Pipe); err != nil {
                return err
            }
        }
        if n.List != nil {
            if err := walkNodes(n.List.Nodes); err != nil {
                return err
            }
        }
        if n.ElseList != nil {
            if err := walkNodes(n.ElseList.Nodes); err != nil {
                return err
            }
        }
    case *parse.WithNode:
        // ... same 15 lines again ...
    case *parse.TemplateNode:
        return fmt.Errorf("{{template}} is not allowed")
    }
    return nil
}
Enter fullscreen mode Exit fullscreen mode

Three cases — RangeNode, IfNode, WithNode — with identical Pipe/List/ElseList validation logic. 45 lines of triplicated nil-check-then-walk. Each case is 15 lines of the same nested if != nil { if err := ...; err != nil { return err } } pattern.

After: Extract shared structure into walkBranch 

func walkNode(node parse.Node) error {
    switch n := node.(type) {
    case *parse.ActionNode:
        return walkPipe(n.Pipe)
    case *parse.RangeNode:
        return walkBranch(n.Pipe, n.List, n.ElseList)
    case *parse.IfNode:
        return walkBranch(n.Pipe, n.List, n.ElseList)
    case *parse.WithNode:
        return walkBranch(n.Pipe, n.List, n.ElseList)
    case *parse.TemplateNode:
        return fmt.Errorf("{{template}} is not allowed")
    }
    return nil
}
Enter fullscreen mode Exit fullscreen mode

The extracted helper:

// walkBranch validates the common Pipe/List/ElseList structure shared by
// if, range, and with nodes.
func walkBranch(pipe *parse.PipeNode, list, elseList *parse.ListNode) error {
    if err := walkPipe(pipe); err != nil {
        return err
    }
    if list != nil {
        if err := walkNodes(list.Nodes); err != nil {
            return err
        }
    }
    if elseList != nil {
        if err := walkNodes(elseList.Nodes); err != nil {
            return err
        }
    }
    return nil
}
Enter fullscreen mode Exit fullscreen mode

45 lines of triplicated logic → 3 one-liners + one 15-line helper. The walkPipe nil check moved into walkPipe itself, eliminating the if pipe != nil guard at every call site.

The Rule

Every pattern follows one principle: the happy path should be at the leftmost indentation level.

Nesting Cause Fix Result
Loop body wrapped in conditional Invert + continue Main logic at loop level
Triple-nested loops Extract inner loop to function Outer function stays shallow
if/else setting a variable Early return per branch No shared mutable variable
Two guards with same outcome Merge into single condition Half the code
Monolith function with branches Split into helpers with early returns Each helper has one concern
Identical branches in a switch Extract shared structure One function, multiple callers

When you see indentation, ask: "is this indentation carrying information, or is it an artifact of control flow?" If the indentation doesn't tell the reader something new, flatten it.

When NOT to Flatten

Not every nested block needs flattening:

// This is fine — the nesting IS the logic
select {
case msg := <-ch:
    if msg.Type == "error" {
        return handleError(msg)
    }
    process(msg)
case <-ctx.Done():
    return ctx.Err()
}
Enter fullscreen mode Exit fullscreen mode

Don't flatten when:

  • The nesting represents genuine decision structure (select/case)
  • Extracting a helper would require passing 6+ parameters
  • The function is already under 15 lines
  • The "flattened" version would require goto or repeated cleanup

Flatten when:

  • The happy path is buried 3+ levels deep
  • An else branch mirrors the if branch
  • Identical blocks appear in multiple switch cases
  • A loop body is entirely wrapped in a single conditional

These 6 patterns were applied across 60 refactorings in Stave, an offline configuration safety evaluator. The gocritic nestingReduce and gocognit linters enforce these patterns at CI time.

Top comments (0)