Clean Architecture · Common pitfalls

1 min read
Mid-level13 min read
Rapid overview

Common pitfalls

1. Anemic Domain Model

Mistake: Entities are just property bags with no behavior. All logic lives in services.

// BAD -- anemic entity
public class Trade
{
    public Guid Id { get; set; }
    public string Symbol { get; set; }
    public TradeStatus Status { get; set; } // public setter!
}

// BAD -- service does what the entity should do
public class TradeService
{
    public void CloseTrade(Trade trade, decimal exitPrice)
    {
        if (trade.Status != TradeStatus.Open)
            throw new Exception("Cannot close");
        trade.Status = TradeStatus.Closed;
    }
}

Fix: Move behavior into the entity. The entity enforces its own invariants.

2. Leaking Infrastructure into the Domain

Mistake: Domain entities have EF Core attributes, JSON attributes, or references to HttpContext.

// BAD -- infrastructure concern in the domain
[Table("trades")]
public class Trade
{
    [Key]
    public Guid Id { get; set; }

    [Column("sym")]
    [JsonPropertyName("symbol")]
    public string Symbol { get; set; }
}

Fix: Use IEntityTypeConfiguration<T> in Infrastructure for EF mapping. Use DTOs for serialization.

3. Application Layer Depends on Infrastructure Types

Mistake: Handlers import AppDbContext, SqlConnection, or vendor-specific types.

// BAD -- handler depends on EF directly
public class OpenTradeHandler
{
    private readonly AppDbContext _db; // Infrastructure type!

    public async Task Handle(OpenTradeCommand cmd, CancellationToken ct)
    {
        _db.Trades.Add(Trade.Open(cmd.Symbol, cmd.Quantity, cmd.EntryPrice));
        await _db.SaveChangesAsync(ct);
    }
}

Fix: Depend on ITradeRepository and IUnitOfWork defined in the Application layer.

4. God Controller

Mistake: The controller contains business logic, validation, and data access.

// BAD -- controller does everything
[HttpPost]
public async Task<IActionResult> Post([FromBody] TradeRequest request)
{
    if (string.IsNullOrEmpty(request.Symbol))
        return BadRequest("Symbol required");

    var trade = new Trade { Symbol = request.Symbol, Quantity = request.Quantity };
    _db.Trades.Add(trade);
    await _db.SaveChangesAsync();

    await _emailService.SendAsync($"Trade {trade.Id} created");
    return Ok(trade);
}

Fix: Controllers should only deserialize input, dispatch to MediatR, and return the response.

5. Shared Models Across Boundaries

Mistake: Using the same class as the API request, the command, the domain entity, and the database row.

Fix: Maintain separate models per boundary. The cost of a few extra record/class declarations is far less than the cost of coupling everything together.

6. Over-Engineering from Day One

Mistake: Implementing full CQRS with separate read/write databases, event sourcing, and message queues before you have a second user.

Fix: Start with a single database, MediatR for in-process CQRS, and grow into separate read stores only when metrics demand it.

7. Circular Dependencies Between Layers

Mistake: Infrastructure references Presentation, or Domain references Application.

Fix: Strict project reference rules enforced by architecture tests (NetArchTest).

8. Not Using CancellationToken

Mistake: Ignoring CancellationToken in async chains, making the app unresponsive to client disconnects.

Fix: Pass CancellationToken through every async method from the controller down to the repository.


See also