Audit this Go codebase and aggressively simplify AI-generated, defensive, overly abstract, or unnecessarily generic code.
The goal is to make the code look like it was written by an experienced Go engineer:
- simple
- explicit
- strongly typed
- boring
- idiomatic
- easy to trace
- minimal abstraction
- minimal magic
Do not optimize for cleverness.
Do not replace simple code with frameworks, generic helpers, reflection, interfaces, factories, adapters, builders, or excessive layering.
The primary rule is:
Validate untrusted data at the boundary. Use concrete types everywhere else.
Search aggressively for:
any
interface{}
map[string]any
map[string]interface{}
Ask why the value is untyped.
Bad:
func getDomain(data map[string]any) string {
value, ok := data["domain"]
if !ok {
return ""
}
domain, ok := value.(string)
if !ok {
return ""
}
return domain
}
Prefer:
type DomainEvent struct {
Domain string `json:"domain"`
}
Then:
event.Domain
Do not carry generic maps through the application and repeatedly recover types from them.
If the JSON shape is known, unmarshal directly into a struct.
Be suspicious of functions like:
toString()
asString()
stringValue()
safeString()
toInt()
asInt()
toBool()
toMap()
asMap()
getString()
getOptionalString()
valueOrDefault()
Bad:
func toString(value any) string {
switch v := value.(type) {
case string:
return v
case int:
return strconv.Itoa(v)
case int64:
return strconv.FormatInt(v, 10)
case fmt.Stringer:
return v.String()
case nil:
return ""
default:
return fmt.Sprintf("%v", v)
}
}
Ask instead:
What type is this value actually supposed to be?
If it is a string:
func normalizeDomain(domain string) string {
return strings.ToLower(strings.TrimSpace(domain))
}
Do not accept any just to make a helper "flexible."
Search for code that silently converts invalid states into:
""
0
false
nil
[]T{}
map[K]V{}
Examples:
if value == nil {
return ""
}
if err != nil {
return nil
}
if project == nil {
return &Project{}
}
Do not hide invalid states.
If data is required, return an error.
Bad:
func projectID(project *Project) string {
if project == nil {
return ""
}
return project.ID
}
Prefer fixing the caller so project cannot be nil there.
Or, if absence is genuinely possible:
if project == nil {
return ErrProjectNotFound
}
Validate/narrow once, then continue with clean code.
Nil checks should correspond to actual nullable states.
Bad:
func handleProject(project *Project) error {
if project == nil {
return errors.New("project is nil")
}
if project.Config == nil {
return errors.New("project config is nil")
}
if project.Config.Domain == nil {
return errors.New("domain is nil")
}
// actual logic
}
If those fields are required by the application model, redesign the types instead.
Prefer:
type Project struct {
Config ProjectConfig
}
type ProjectConfig struct {
Domain string
}
Then:
project.Config.Domain
Do not represent required values as pointers merely because Go allows pointers.
Review struct fields like:
*string
*bool
*int
*time.Time
Do not use pointers merely to distinguish "missing" from zero unless the distinction actually matters.
Bad:
type Config struct {
Enabled *bool
Port *int
Name *string
}
If those values are required:
type Config struct {
Enabled bool
Port int
Name string
}
Pointers should communicate real optionality or identity/mutation semantics.
Do not create a helper for every 2–3 lines.
Be suspicious of:
getDomainID()
extractProjectID()
resolveName()
buildKey()
parseValue()
stringFrom()
domainFrom()
when the helper:
- has one caller
- only accesses a field
- only checks nil
- only calls
strings.TrimSpace - only calls
.String() - only performs a type assertion
- only forwards arguments
Bad:
func domainIDFrom(record *DNSRecord) string {
if record == nil {
return ""
}
return record.DomainID.String()
}
Prefer:
record.DomainID.String()
provided record is already known to exist.
A helper should represent a real reusable concept, not hide straightforward code.
Go interfaces should usually be defined by the consumer and kept small.
Be suspicious of:
type ProjectService interface {
CreateProject(...)
GetProject(...)
UpdateProject(...)
DeleteProject(...)
ListProjects(...)
ValidateProject(...)
SyncProject(...)
RefreshProject(...)
}
especially if there is only one implementation.
Do not create an interface merely because "services should have interfaces."
Prefer concrete types unless you genuinely need:
- multiple implementations
- substitution
- testing at that boundary
- plugin behavior
- a narrow consumer contract
Bad:
type DomainManager interface {
Map(...)
}
with only:
type domainManagerImpl struct{}
Prefer:
type DomainManager struct{}
Do not create Java-style IFoo / FooImpl architecture in Go.
When an interface is justified, define only what the consumer needs.
Bad:
type Repository interface {
Create(...)
Update(...)
Delete(...)
Get(...)
List(...)
Count(...)
Exists(...)
Search(...)
}
when a component only needs:
Get(...)
Prefer:
type projectGetter interface {
Get(ctx context.Context, id string) (*Project, error)
}
Do not expose giant interfaces just because a concrete repository has many methods.
Look for chains like:
handler
↓
controller
↓
service
↓
manager
↓
processor
↓
repository
↓
store
↓
database client
where most layers simply forward arguments.
Bad:
func (s *Service) GetProject(ctx context.Context, id string) (*Project, error) {
return s.manager.GetProject(ctx, id)
}
and:
func (m *Manager) GetProject(ctx context.Context, id string) (*Project, error) {
return m.repository.GetProject(ctx, id)
}
If these layers do not contain meaningful business logic, remove them.
One meaningful layer is better than five pass-through layers.
Search for functions whose entire body is:
return dependency.Do(...)
or:
return helper(value)
or:
result, err := dependency.Do(...)
if err != nil {
return nil, err
}
return result, nil
Simplify:
return dependency.Do(...)
Do not add wrappers purely to make the architecture look layered.
Bad:
result, err := repo.Get(ctx, id)
if err != nil {
return nil, err
}
return result, nil
Prefer:
return repo.Get(ctx, id)
Bad:
if err != nil {
return fmt.Errorf("error: %w", err)
}
This adds no useful context.
If wrapping, add meaningful context:
if err != nil {
return fmt.Errorf("load project %s: %w", id, err)
}
But do not mechanically wrap every error at every layer.
An error should not become:
failed to process project:
failed to get project:
failed to retrieve project:
failed to query project:
sql: no rows
Add context where it materially improves debugging.
Find:
if err != nil {
return nil
}
or:
if err != nil {
log.Println(err)
return nil
}
or:
_ = doSomething()
Determine whether ignoring the error is intentional.
Do not turn bugs into silent success.
If an error can safely be ignored, make the reasoning obvious.
Example:
if err := cache.Delete(ctx, key); err != nil {
logger.Warn("failed to invalidate cache", "key", key, "error", err)
}
only when cache invalidation failure genuinely should not fail the operation.
Be suspicious of enormous error hierarchies.
Do not create:
type ValidationError struct{}
type RepositoryError struct{}
type ServiceError struct{}
type DomainError struct{}
type InternalError struct{}
unless callers actually need different behavior based on the error.
Prefer:
var ErrProjectNotFound = errors.New("project not found")
and:
errors.Is(err, ErrProjectNotFound)
Use structured custom errors only when they carry meaningful information.
Do not manually inspect error strings.
Bad:
if strings.Contains(err.Error(), "duplicate") {
Prefer the underlying driver's supported error type/code.
Example:
var writeErr mongo.WriteException
if errors.As(err, &writeErr) {
...
}
But do not turn a simple known-driver check into enormous generic error inspection code.
Keep it proportional to the actual need.
Search for:
reflect.
Reflection is suspicious in ordinary business logic.
Bad:
func isEmpty(value any) bool {
v := reflect.ValueOf(value)
...
}
Prefer explicit typed logic.
Do not use reflection to avoid writing five obvious lines.
Reflection is reasonable in areas such as:
- serializers
- frameworks
- generic libraries
- tooling
It should rarely appear in normal handlers/services/domain logic.
Go generics are useful, but AI often introduces them for trivial problems.
Be suspicious of:
func Ptr[T any](value T) *T
func ValueOrDefault[T comparable](...)
func ConvertSlice[T any, R any](...)
func SafeCast[T any](...)
func GetOrDefault[K comparable, V any](...)
Do not create generic utilities just because two lines look similar.
Prefer concrete domain code where it is easier to understand.
A generic abstraction should solve a real recurring problem.
Bad:
func GetString(data map[string]any, key string) string
func GetInt(data map[string]any, key string) int
func GetBool(data map[string]any, key string) bool
This is usually evidence that structured data should have been decoded into a struct.
Prefer:
type DeploymentEvent struct {
ProjectID string `json:"project_id"`
Port int `json:"port"`
Enabled bool `json:"enabled"`
}
Then use:
event.ProjectID
event.Port
event.Enabled
Bad:
var payload map[string]any
if err := json.Unmarshal(body, &payload); err != nil {
return err
}
event, _ := payload["event"].(string)
data, _ := payload["data"].(map[string]any)
projectID, _ := data["project_id"].(string)
Prefer:
type QueueEvent struct {
Event string `json:"event"`
Data json.RawMessage `json:"data"`
}
Then decode the event-specific payload once:
type ProjectSyncEvent struct {
ProjectID string `json:"project_id"`
}
var event ProjectSyncEvent
if err := json.Unmarshal(payload.Data, &event); err != nil {
return fmt.Errorf("decode project sync event: %w", err)
}
After decoding, business logic should operate on typed structs.
If MongoDB, JSONB, Redis, or another store returns known document shapes, define structs.
Bad:
var document map[string]any
followed by:
id, ok := document["domain"].(primitive.ObjectID)
Prefer:
type DNSDocument struct {
Domain primitive.ObjectID `bson:"domain"`
}
Then:
document.Domain.Hex()
Fix broad types at the data-access boundary rather than adding extractors everywhere.
Bad:
value, ok := data.(map[string]any)
if !ok {
return nil
}
domain, ok := value["domain"].(string)
if !ok {
return nil
}
if the data came from a contract already controlled by the application.
Either:
- type it correctly upstream, or
- genuinely validate it at the external boundary
Do not repeatedly rediscover types inside trusted application code.
Be suspicious when the same data has:
ProjectRequest
ProjectDTO
ProjectInput
ProjectParams
ProjectData
ProjectModel
ProjectEntity
ProjectResponse
with nearly identical fields.
Separate types when the contracts are materially different.
Do not duplicate structs solely because each layer "needs its own model."
If two layers genuinely share the same concept, use the same type.
Look for functions like:
func projectToDTO(project Project) ProjectDTO {
return ProjectDTO{
ID: project.ID,
Name: project.Name,
}
}
when ProjectDTO and Project are effectively identical and no boundary requires the distinction.
Do not maintain fleets of:
toDTO
fromDTO
toModel
fromModel
toEntity
fromEntity
without a meaningful difference in representation.
Bad:
func NewProjectService(repo Repository) *ProjectService {
return &ProjectService{
repo: repo,
}
}
This constructor can be reasonable if it provides a stable construction API.
But do not add constructors for simple data structs:
func NewDomain(name string) Domain {
return Domain{Name: name}
}
when:
Domain{Name: name}
is clearer.
Constructors should establish invariants or hide meaningful setup.
Do not introduce Java-style builders for simple structs.
Bad:
deployment := NewDeploymentBuilder().
WithProjectID(projectID).
WithRegion(region).
WithPort(port).
WithImage(image).
Build()
Prefer:
deployment := Deployment{
ProjectID: projectID,
Region: region,
Port: port,
Image: image,
}
Builders are justified only when construction is genuinely complex.
Avoid:
NewService(
WithRepository(repo),
WithLogger(logger),
WithMetrics(metrics),
)
when all fields are required.
Prefer:
NewService(repo, logger, metrics)
Functional options are useful primarily for optional configuration or APIs with many optional settings.
Do not introduce them just because they are a popular Go pattern.
Bad:
type RepositoryFactory struct{}
func (f *RepositoryFactory) CreateRepository(kind string) Repository
when the application has one concrete repository.
Prefer constructing the concrete dependency directly.
Factories should exist because runtime selection is real, not because "factory pattern" sounds architectural.
Normal Go dependency injection is usually just:
service := NewService(repo, logger)
Do not introduce:
- containers
- service locators
- registries
- providers
- dependency graphs
- reflection-based injection
unless the project genuinely needs them.
Explicit wiring is a strength of Go.
Bad:
var shouldProcess bool
if project != nil {
if project.Enabled {
if project.Status == StatusActive {
shouldProcess = true
}
}
}
Prefer early returns:
if project == nil {
return nil
}
if !project.Enabled {
return nil
}
if project.Status != StatusActive {
return nil
}
// actual work
Or when simple:
shouldProcess := project != nil &&
project.Enabled &&
project.Status == StatusActive
Choose whichever is easier to read.
Do not optimize for clever one-liners.
Bad:
if err == nil {
if project != nil {
if project.Enabled {
// 80 lines
}
}
}
Prefer:
if err != nil {
return err
}
if project == nil {
return ErrProjectNotFound
}
if !project.Enabled {
return nil
}
// main logic
Keep the happy path visually obvious.
Bad:
rawDomain := event.Domain
normalizedDomain := strings.TrimSpace(rawDomain)
domain := strings.ToLower(normalizedDomain)
Prefer:
domain := strings.ToLower(strings.TrimSpace(event.Domain))
But do not compress code so aggressively that readability decreases.
Intermediate variables should represent meaningful concepts.
Clean up:
if enabled == true
if enabled == false
Prefer:
if enabled
if !enabled
Unless an API uses nullable booleans where the distinction matters.
Question code like:
items := make([]Item, 0)
when:
var items []Item
is sufficient.
Likewise do not create:
make(map[string]string)
until the map actually needs writes.
But keep capacity preallocation where profiling/data size makes it useful.
Be suspicious of defensive copying with no mutation threat.
Bad:
result := make([]string, len(input))
copy(result, input)
return result
unless ownership/mutation semantics require the copy.
Do not add allocations "for safety" without a concrete reason.
Do not introduce:
sync.Pool- manual buffer reuse
- unsafe conversions
- custom allocators
- elaborate caches
- goroutine pools
- lock-free structures
without evidence that the code needs them.
Simple correct code first.
Performance optimizations should solve measured problems.
Be suspicious of:
go func() {
...
}()
added merely to make something "non-blocking."
Every goroutine introduces:
- lifecycle concerns
- cancellation concerns
- race potential
- error propagation problems
- shutdown complexity
Use concurrency when the operation actually benefits from concurrency.
Bad:
handler
↓
channel
↓
worker
↓
channel
↓
processor
for logic that could simply be:
processor.Process(ctx, event)
Channels are synchronization primitives, not an architectural requirement.
Do not use them to make normal function calls look concurrent.
Do not:
- store context permanently on structs
- create
context.Background()deep in request processing - accept
context.Contextwhere cancellation/deadlines are irrelevant - nil-check context
Bad:
if ctx == nil {
ctx = context.Background()
}
A context parameter should not be nil.
Pass the caller's context through I/O boundaries.
Typical signature:
func (s *Service) GetProject(ctx context.Context, id string) (*Project, error)
Do not create new contexts just to satisfy a function signature.
Do not mechanically write:
ctx, cancel := context.WithTimeout(ctx, 5*time.Second)
defer cancel()
inside every repository/service function.
Timeout policy should usually live at meaningful boundaries.
Repeated nested arbitrary timeouts are difficult to reason about.
Avoid logs that merely narrate every function:
logger.Info("entering CreateProject")
logger.Info("validating project")
logger.Info("calling repository")
logger.Info("repository completed")
logger.Info("leaving CreateProject")
Log meaningful events:
- failures
- important state transitions
- operational decisions
- external interactions worth tracing
Do not turn application logs into execution commentary.
Bad:
result, err := repo.Get(ctx, id)
if err != nil {
logger.Error("failed to get project", "error", err)
return nil, err
}
if the caller will also log it.
Prefer logging once at the boundary responsible for handling the failure.
Libraries/services should generally return errors.
Handlers/workers/process supervisors decide when to log.
Delete comments like:
// Check if project exists.
if project == nil {
// Return the result.
return result
// Convert string to lowercase.
domain = strings.ToLower(domain)
Keep comments for:
- business rules
- invariants
- non-obvious decisions
- external system quirks
- workarounds
- concurrency reasoning
Comments should explain why, not narrate syntax.
Do not create a package for every type/helper.
Bad:
internal/
domainparser/
stringutils/
validationhelper/
projectmapper/
pointerhelper/
responsebuilder/
Prefer packages around actual domains/capabilities.
A package should represent a coherent concept, not one function.
Audit packages named:
utils
helpers
common
shared
misc
core
base
These often accumulate unrelated abstractions.
Move useful functions to the domain that owns them.
Delete trivial helpers.
Avoid creating another generic utility package during cleanup.
Bad:
type ProjectID struct {
Value string
}
when a plain string is sufficient.
A custom type may be appropriate:
type ProjectID string
if it prevents mixing IDs or adds domain behavior.
But do not wrap primitives in structs without a concrete benefit.
This can be useful:
type ProjectID string
type Region string
when it prevents accidental mixing.
But do not produce:
type ProjectName string
type ProjectDescription string
type ProjectImage string
type ProjectStatusString string
for every field.
Use domain types where they materially improve correctness.
Before keeping a helper, check whether Go already has the operation.
Prefer:
strings.TrimSpace
strings.ToLower
slices.Contains
maps.Clone
errors.Is
errors.As
cmp.Or
strconv.Atoi
where appropriate.
Do not maintain custom helpers that poorly reimplement the standard library.
Bad:
regexp.MustCompile(`\s+`).ReplaceAllString(...)
for simple trimming or known delimiters.
Prefer strings functions where sufficient.
Regex should solve regex-shaped problems.
Avoid:
fmt.Sprintf("%s", value)
when value is already a string.
Avoid:
fmt.Sprintf("%d", n)
in hot/simple paths when:
strconv.Itoa(n)
is clearer.
But do not replace readable formatting merely for micro-performance.
Bad:
func validateProject(project Project) error {
if project.ID == "" {
return errors.New("missing project ID")
}
...
}
called in every internal service.
If Project is created from external input, validate when creating/parsing it.
Do not repeatedly validate the same object throughout the system.
Do not blindly remove validation.
Validation is appropriate for:
- HTTP requests
- query/path parameters
- queue messages
- webhooks
- config/env vars
- external APIs
- user input
- decoded untrusted JSON
- persisted schemaless documents
The rule is:
Defend against external uncertainty, not against your own correctly typed code.
If normal Go code is sufficient:
if req.Domain == "" {
return ErrDomainRequired
}
do not introduce a large validation framework solely to avoid three if statements.
Use an existing validation library if the project already relies on it and the schema complexity warrants it.
Do not hide meaningful rules behind generic abstractions.
Bad:
if validator.IsValid(project) {
when the real business rule is:
if project.Status != StatusActive {
return ErrProjectInactive
}
Domain rules should be visible in the code.
Bad:
func Process(ctx context.Context, cfg Config, options Options, metadata Metadata)
when the function needs:
projectID
region
Pass the data the function actually needs.
Narrow function signatures improve readability and testability.
Bad:
type GetProjectOptions struct {
ID string
}
for:
GetProject(ctx, GetProjectOptions{ID: id})
Prefer:
GetProject(ctx, id)
Option structs are useful when several meaningful parameters travel together or optional parameters exist.
Bad:
type ExistsResult struct {
Exists bool
}
Prefer:
func Exists(...) (bool, error)
Use structs when multiple related return values form a meaningful object.
Bad:
func GetProject(id string) (project *Project, err error) {
...
}
unless named returns materially improve the function.
Prefer:
func GetProject(id string) (*Project, error) {
Avoid naked returns in non-trivial functions.
Search for:
defer func() {
if r := recover(); r != nil {
...
}
}()
Do not use recover to turn programming bugs into normal control flow.
Recover only at genuine process/request boundaries where keeping the process alive is intentional.
Do not put recover() inside normal business functions.
Do not use panic for:
- validation failures
- missing DB rows
- network errors
- malformed user input
Return errors.
Panics are appropriate for states that make program initialization or continued execution impossible.
Review:
var defaultClient ...
var globalConfig ...
var singleton ...
Globals are fine for true constants/immutable package state.
Do not use mutable package globals as a shortcut for dependency wiring.
Pass dependencies explicitly.
Bad:
Service
→ Repository
→ Store
→ DAO
→ QueryExecutor
→ sql.DB
Use the minimum layering that matches the application.
A repository around SQL/Mongo queries is reasonable.
A repository wrapped by another object that just forwards everything is not.
Do not create elaborate query builders for three static queries unless dynamic composition is actually needed.
Plain SQL is often easier to understand:
const query = `
SELECT id, name
FROM projects
WHERE id = $1
`
Do not hide straightforward SQL behind abstraction solely to avoid writing SQL.
Do not invent elaborate:
TransactionManager
UnitOfWork
TransactionProvider
TransactionRunner
if this suffices:
tx, err := db.BeginTx(ctx, nil)
if err != nil {
return err
}
defer tx.Rollback()
...
return tx.Commit()
Abstract transaction handling only when repeated complexity justifies it.
Do not replace every string with a constant.
Good:
const duplicateKeyCode = 11000
when the number has domain/driver meaning.
Unnecessary:
const emptyString = ""
const trueValue = true
Constants should communicate meaning.
Custom string constants are useful:
type DeploymentStatus string
const (
DeploymentPending DeploymentStatus = "pending"
DeploymentRunning DeploymentStatus = "running"
)
But do not create enums for every arbitrary string if there is no closed set of valid values.
Go switch is often better than abstraction.
Bad:
handlers := map[string]func(Event) error{
"insert": handleInsert,
"delete": handleDelete,
}
when a simple switch is clearer:
switch event.Type {
case "insert":
return handleInsert(event)
case "delete":
return handleDelete(event)
default:
return ErrUnsupportedEvent
}
Use dispatch maps when dynamic registration is genuinely valuable.
If there are only two slightly different cases, a switch may be clearer than:
type Strategy interface {
Execute(...)
}
plus:
InsertStrategy
DeleteStrategy
ReplaceStrategy
Go does not require every branch to become polymorphism.
Do not create interfaces purely so every dependency can be mocked.
Prefer testing real behavior where practical.
Use small interfaces around expensive/external boundaries.
Do not turn every internal type into an interface because "tests need mocks."
Apply the cleanup to tests.
Remove:
- enormous test builders
- generic fixture systems
- helper pyramids
- mocks for pure logic
- repeated
any - excessive test abstractions
Prefer explicit table-driven tests where appropriate:
tests := []struct {
name string
in string
want string
}{
{"lowercase", "EXAMPLE.COM", "example.com"},
{"trim", " example.com ", "example.com"},
}
Do not make tests harder to understand than the code they test.
This cleanup must not mistake normal Go code for slop.
This is idiomatic and should remain:
if err != nil {
return err
}
Likewise:
value, ok := m[key]
if !ok {
...
}
and:
if project == nil {
...
}
can all be correct.
The question is whether the failure/absence is genuinely possible and meaningful.
Do not remove necessary checks merely to reduce line count.
The desired code should be shorter because unnecessary concepts disappeared.
Not because everything was compressed into unreadable expressions.
Bad cleanup:
if err := func() error { ... }(); err != nil { return fmt.Errorf(...) }
Prefer boring readable Go.
Whenever you see:
asString(value)
asObjectID(value)
extractDomain(value)
safeValue(value)
toMap(value)
trace where value originated.
Ask:
- Why isn't this value already strongly typed?
- Is this data external?
- Can it be decoded into a concrete struct at the boundary?
- Can downstream functions accept the concrete type?
- Can this helper then disappear?
Always prefer fixing the source of poor typing.
Be particularly suspicious of combinations like:
interfaces.go
factory.go
builder.go
mapper.go
converter.go
validator.go
utils.go
helpers.go
manager.go
processor.go
service.go
repository.go
inside one small feature.
Do not assume all these layers are necessary.
Determine what each one actually does.
Collapse layers that merely forward calls or transform identical structures.
Search the repository for:
any
interface{}
map[string]any
map[string]interface{}
reflect.
recover(
panic(
fmt.Sprintf
strconv.
errors.New
fmt.Errorf
errors.As
errors.Is
== nil
!= nil
type .* interface
Factory
Builder
Manager
Processor
Helper
Utils
Mapper
Converter
Validator
Options
Params
DTO
Entity
Model
ValueOr
Safe
Normalize
Extract
Resolve
Parse
GetString
AsString
ToString
With
context.Background
context.TODO
sync.Pool
go func
make([]
Do not automatically change every match.
Use them as places to inspect for unnecessary complexity.
Before adding or keeping code, ask:
Does this handle something that can genuinely happen?
If no, delete it.
Ask:
Is this complexity caused by poor typing upstream?
If yes, fix the upstream type.
Ask:
Does this interface have more than one meaningful implementation or consumer-driven purpose?
If no, consider using the concrete type.
Ask:
Does this helper express a real concept?
If no, inline it.
Ask:
Does this abstraction reduce total complexity?
If no, remove it.
Ask:
Would plain Go be easier to understand?
If yes, use plain Go.
Prefer code like:
func (s *Service) MapDomain(ctx context.Context, event DomainMapEvent) error {
domain := strings.ToLower(strings.TrimSpace(event.Domain))
return s.domains.Map(ctx, event.ProjectID, domain)
}
over:
func (s *Service) MapDomain(ctx context.Context, raw any) error {
event, err := convertToDomainMapEvent(raw)
if err != nil {
return fmt.Errorf("failed converting domain event: %w", err)
}
projectID := safeString(event.ProjectID)
if projectID == "" {
return nil
}
domain := normalizeStringValue(event.Domain)
if domain == "" {
return nil
}
return s.domainManager.ProcessDomainMapping(
ctx,
NewDomainMappingParams(projectID, domain),
)
}
Work feature-by-feature.
For each feature:
Identify where data enters the system.
Examples:
- HTTP
- queue
- Kafka/NATS/RabbitMQ
- MongoDB
- PostgreSQL
- Redis
- webhook
- external API
- environment/config
Give the input a concrete type.
Validate/decode once.
Follow the data through the application.
Remove unnecessary:
any- type assertions
- generic maps
- nil guards
- converters
- extractors
- wrapper structs
- interfaces
- pass-through methods
- duplicate DTOs
- generic helpers
Collapse forwarding layers.
Delete dead abstractions.
Run tests and static analysis.
Use the project's normal commands, including where applicable:
go test ./...
go vet ./...
staticcheck ./...
Run formatters after changes:
gofmt
Do not change behavior merely to satisfy style preferences.
Do not remove:
- useful interfaces
- meaningful error wrapping
- legitimate nil handling
- boundary validation
- context propagation
- proper resource cleanup
defer rows.Close()defer resp.Body.Close()- transaction rollback safety
- mutexes protecting real shared state
- channel synchronization that is actually needed
- driver-specific error handling
- correct integer/error checks
- security-related checks
This is a complexity cleanup, not reckless deletion.
The codebase should end up with:
- more concrete structs
- fewer
anyvalues - fewer generic maps
- fewer type assertions
- fewer conversion helpers
- fewer tiny wrapper functions
- fewer pointless interfaces
- fewer forwarding layers
- fewer factories/builders/managers
- less reflection
- less defensive nil handling
- less silent fallback behavior
- simpler error handling
- fewer redundant DTOs
- more direct function calls
- narrower function signatures
- explicit business logic
- validation concentrated at boundaries
- straightforward idiomatic Go
The important metric is not the number of files changed.
The important metric is:
Can an engineer trace the behavior without jumping through unnecessary abstractions?
Do not replace one kind of slop with another.
Do not turn:
value := data["domain"].(string)
into:
value, ok := data["domain"]
if !ok {
return ""
}
domain, ok := value.(string)
if !ok {
return ""
}
and call that a cleanup.
The correct solution is usually:
type Event struct {
Domain string `json:"domain"`
}
followed by:
event.Domain
Likewise, do not replace a simple direct call with:
interface
→ implementation
→ manager
→ helper
→ converter
→ validator
→ actual function
The overriding principle is:
Make untrusted input safe at the edge. Keep trusted Go code boring everywhere else.