Refactoring

Refactoring is changing the structure of code without changing what it does. It is not a rewrite, not a feature, and not a two-week "cleanup sprint" — done well, it's a series of small steps, each of which leaves the code working, mixed continuously into everyday work. This lesson covers why and when to refactor, the smells that signal a problem, how to make legacy code safe to change, the core refactorings with Go examples, and how to fit refactoring into real delivery.

#What refactoring is — and isn't

Martin Fowler's definition has two parts: a change to the internal structure of software to make it easier to understand and cheaper to modify, without changing its observable behaviour. The second half is the discipline. If a commit renames a function and fixes a bug and adds a field, a reviewer can't tell which diff lines are the risky ones. Kent Beck's advice captures the rhythm:

For each desired change, make the change easy (warning: this may be hard), then make the easy change.

So the workflow for most features is: refactor so the new feature has an obvious home, commit, then add the feature, commit. Two small reviewable diffs instead of one tangled one.

Refactor when:

• You're about to change code and its shape makes the change hard (preparatory refactoring).
• You've just understood code that took effort to read — encode that understanding in better names and structure before you lose it.
• You see the same fix needed for the third time (rule of three).

Don't refactor code you won't touch again just because it offends you, and don't refactor without tests.

#Code smells

A smell is a surface sign that suggests a deeper problem. It's a prompt to look closer, not a rule. The ones you'll meet most:

The code smells note is a quick reference. Notice how many smells are coupling and cohesion problems in disguise: shotgun surgery is low cohesion, feature envy is misplaced responsibility.

#Make it safe first: characterization tests

You can't refactor safely without tests that tell you when behaviour changed. Legacy code often has none, and you may not even know what the "correct" behaviour is. Characterization tests pin down what the code currently does — bugs included — so you can restructure with confidence:

1. Call the code with a representative input.
2. Write an assertion you know is wrong, run it, and read the actual output from the failure.
3. Change the assertion to the actual output. Repeat for edge cases.

func TestLegacyInvoiceTotal_Characterization(t *testing.T) {
	tests := []struct {
		name  string
		items []LegacyItem
		want  string // whatever the code produces today
	}{
		{"single item", []LegacyItem{{"A", 2, 15000}}, "30000.00"},
		{"empty", nil, "0.00"},
		{"discount line", []LegacyItem{{"A", 1, 10000}, {"DISC", 1, -2000}}, "8000.00"},
		{"negative quantity", []LegacyItem{{"A", -1, 10000}}, "-10000.00"}, // a bug? pinned for now
	}
	for _, tt := range tests {
		t.Run(tt.name, func(t *testing.T) {
			if got := LegacyInvoiceTotal(tt.items); got != tt.want {
				t.Errorf("LegacyInvoiceTotal() = %q; want %q", got, tt.want)
			}
		})
	}
}

The "negative quantity" case is probably a bug, but refactoring isn't the moment to fix it. Pin it, finish the refactor, then fix the bug in a separate commit with a test that states the intended behaviour.

When code is too tangled to call in a test — it opens database connections in the middle, reads globals, or calls time.Now() — find a seam: a place where you can change behaviour without editing the code there. In Go, the usual seam is to extract the dependency into a parameter or an interface field, with the old behaviour as the default, so production callers are unaffected while tests can substitute a fake.

#The core refactorings

A small set of moves covers most day-to-day refactoring. Each is behaviour-preserving if done mechanically, and modern editors (gopls in VS Code or GoLand, the TypeScript language server) automate several: rename, extract function, extract variable, inline, move.

Extract function. Turn a fragment into a function named after what it does:

// Before
func (s *Service) Notify(ctx context.Context, u User, msg string) error {
	// format the phone number
	phone := strings.ReplaceAll(u.Phone, " ", "")
	if strings.HasPrefix(phone, "0") {
		phone = "+62" + phone[1:]
	}
	return s.sms.Send(ctx, phone, msg)
}

// After
func (s *Service) Notify(ctx context.Context, u User, msg string) error {
	return s.sms.Send(ctx, normalizeIndonesianPhone(u.Phone), msg)
}

func normalizeIndonesianPhone(raw string) string {
	phone := strings.ReplaceAll(raw, " ", "")
	if strings.HasPrefix(phone, "0") {
		phone = "+62" + phone[1:]
	}
	return phone
}

The extracted function is now independently testable, and the comment became a name.

Rename. The cheapest, highest-value refactoring. When you understand what data2 really holds, rename it everywhere — with the editor's rename, not find-and-replace.

Introduce parameter object / extract type. When start, end time.Time travel together through ten functions, make a DateRange with its own methods (Contains, Overlaps). Behaviour that was scattered across callers moves onto the type.

Replace primitive with value type. A Money type that stores minor units as int64 and carries a currency removes a whole class of float-rounding and currency-mixing bugs. See value objects.

Move function. Move a function to the type or package whose data it uses most (fixing feature envy).

Replace conditional with polymorphism. When the same switch order.Type appears in pricing, shipping and invoicing, give each type an implementation of one interface, as shown in the SOLID lesson.

Inline. The reverse of extract. When an abstraction no longer earns its keep — a wrapper that only delegates, a helper with five flags — inline it back into its callers.

#Small steps, always green

The defining technique of refactoring is the step size. Each step should be small enough that if tests fail, you know exactly what caused it, and can undo it in seconds. Run the tests after every step; commit whenever they're green. If tests go red and you don't immediately see why, revert to the last green commit and take a smaller step — it's faster than debugging a half-finished restructure.

For changes too large for one sitting, use parallel change (also called expand–contract): add the new form alongside the old, migrate callers one at a time, then remove the old form. Every intermediate commit compiles and passes. Renaming a widely used function this way looks like: add NewName that calls OldName's body, mark OldName deprecated with a // Deprecated: comment (Go tools flag uses), move callers over in several PRs, then delete OldName. The same idea applies to database schemas — see expand–contract migrations.

For replacing a whole subsystem, the strangler fig pattern routes a growing share of traffic from the old implementation to the new one behind a stable interface, until the old one can be deleted. It's how large rewrites ship without a big-bang cutover.

#Refactoring in a team

• Separate refactoring commits from behaviour changes. Reviewers can skim a pure rename and focus on the commit that changes logic. Say "no behaviour change" in the commit message, and mean it.
• Leave the campsite a little cleaner than you found it — but stay in scope. A bug fix PR that also reformats three unrelated packages is hard to review and harder to revert.
• Don't ask permission to do your job well. Small refactoring is part of building features, not a separate line item. Large restructures that take days do need a plan and buy-in; write a short design doc and deliver them incrementally.
• Measure by outcome: the next change in this area should be faster and less risky. If it isn't, the refactoring didn't help.

#Practice

The function below has several smells. List them, then describe a sequence of small refactoring steps (each leaving tests green) to improve it. Write the final code.

func Report(orders []Order, from, to time.Time, currency string, includeRefunds bool) string {
	total := 0.0
	n := 0
	for _, o := range orders {
		if o.CreatedAt.After(from) && o.CreatedAt.Before(to) && o.Currency == currency {
			if o.Status == "refunded" && !includeRefunds {
				continue
			}
			total += o.Amount
			n++
		}
	}
	avg := 0.0
	if n > 0 {
		avg = total / float64(n)
	}
	return fmt.Sprintf("%d orders, total %.2f %s, average %.2f", n, total, currency, avg)
}

const StatusRefunded = "refunded"

type DateRange struct{ From, To time.Time }

func (r DateRange) Contains(t time.Time) bool { return t.After(r.From) && t.Before(r.To) }

type ReportFilter struct {
	Period         DateRange
	Currency       string
	IncludeRefunds bool
}

func (f ReportFilter) Matches(o Order) bool {
	if !f.Period.Contains(o.CreatedAt) || o.Currency != f.Currency {
		return false
	}
	return f.IncludeRefunds || o.Status != StatusRefunded
}

type Summary struct {
	Count          int
	Total, Average float64
}

func Summarize(orders []Order, f ReportFilter) Summary {
	var s Summary
	for _, o := range orders {
		if f.Matches(o) {
			s.Count++
			s.Total += o.Amount
		}
	}
	if s.Count > 0 {
		s.Average = s.Total / float64(s.Count)
	}
	return s
}

func Report(orders []Order, f ReportFilter) string {
	s := Summarize(orders, f)
	return fmt.Sprintf("%d orders, total %.2f %s, average %.2f", s.Count, s.Total, f.Currency, s.Average)
}

#Key takeaways

• Refactoring changes structure, never behaviour; keep it separate from feature and bug-fix commits.
• Make the change easy, then make the easy change: refactor in preparation for the feature you're about to build.
• Smells are prompts, not rules: long functions, data clumps, primitive obsession, feature envy, shotgun surgery.
• Pin legacy behaviour with characterization tests before changing it, and find seams to get it under test.
• Take small steps, run tests after each, commit when green, and revert rather than debug a broken restructure.
• Use parallel change and the strangler fig to evolve large code without a big-bang rewrite.