Common Clean Architecture Mistakes in .NET (And How to Fix Them)

December 22, 2025 · 5176 words · 23 min read

Contents

Clean Architecture .NET Software Architecture Best Practices Antipatterns

You’ve read the books, watched the tutorials, and decided to implement Clean Architecture in your .NET project. Six months later, your “clean” architecture feels… messy. Sound familiar?

Here’s the thing: Clean Architecture promises maintainability, testability, and flexibility. But I’ve seen countless teams (including my own) turn it into a nightmare of over-engineering, broken abstractions, and tangled dependencies.

Let me share the 7 biggest mistakes I’ve made (and seen) when implementing Clean Architecture in .NET, and more importantly, how to fix them.

New to Clean Architecture? Start with these guides first:
Clean Architecture in .NET Overall
Clean Architecture Real-World Example

Why Even Good Developers Get This Wrong

Clean Architecture isn’t hard to understand conceptually. The dependency rule? Simple. Entities shouldn’t know about databases? Makes sense.

But in practice, it’s easy to slip up when deadlines loom:

“Just this once” - breaking the Dependency Rule
“We’ll add behavior later” - creating anemic domain models
“Let’s be thorough” - over-engineering simple features
“It’s just a reference” - leaking infrastructure concerns into domain
“Everyone uses generic repos” - misusing repositories and DTOs

Let’s fix these, one by one.

Mistake #1: Breaking the Dependency Rule

The Problem: “Just One Little Using Statement…”

This is the cardinal sin of Clean Architecture. The most fundamental rule is: Dependencies point inward, always. Inner layers (Domain) should NEVER know about outer layers (Infrastructure).

Yet I see this constantly:

“I’ll just add using Microsoft.EntityFrameworkCore to my domain entity. What’s the harm?”

The harm? You’ve just coupled your business logic to a database library. Good luck switching to Dapper, or testing without a database, or… you get the idea.

Bad Example: Domain Depending on Infrastructure (Don’t Do This)

// WRONG: Domain/Entities/Order.cs
using Microsoft.EntityFrameworkCore;  // Infrastructure dependency!
using Infrastructure.Persistence;      // Outer layer reference!

public class Order
{
    public Guid Id { get; set; }
    public string Status { get; set; }
    
    // Domain entity knows about DbContext
    public async Task SaveToDatabase(ApplicationDbContext context)
    {
        context.Orders.Update(this);
        await context.SaveChangesAsync();
    }
    
    // Domain entity knows about specific data access
    public static async Task<Order?> LoadFromDatabase(
        ApplicationDbContext context, 
        Guid id)
    {
        return await context.Orders.FindAsync(id);
    }
}

Why This Is Wrong:

Domain layer now depends on Entity Framework Core
Can’t change database without modifying domain
Can’t test domain logic without database
Violates Dependency Inversion Principle

Correct Approach: Pure Domain with Repository Abstraction

// CORRECT: Domain/Entities/Order.cs
// No infrastructure dependencies!
public class Order
{
    public Guid Id { get; private set; }
    public OrderStatus Status { get; private set; }
    public DateTime CreatedAt { get; private set; }
    
    private readonly List<OrderItem> _items = new();
    public IReadOnlyList<OrderItem> Items => _items.AsReadOnly();
    
    private Order() { } // For EF Core
    
    public static Order Create(Guid customerId)
    {
        return new Order
        {
            Id = Guid.NewGuid(),
            Status = OrderStatus.Pending,
            CreatedAt = DateTime.UtcNow
        };
    }
    
    public void AddItem(Product product, int quantity)
    {
        if (Status != OrderStatus.Pending)
            throw new InvalidOperationException("Cannot modify confirmed order");
            
        _items.Add(new OrderItem(product, quantity));
    }
    
    public void Confirm()
    {
        if (!_items.Any())
            throw new InvalidOperationException("Cannot confirm empty order");
            
        Status = OrderStatus.Confirmed;
    }
}

// Application layer defines interface
// Application/Interfaces/IOrderRepository.cs
public interface IOrderRepository
{
    Task<Order?> GetByIdAsync(Guid id);
    Task AddAsync(Order order);
    Task UpdateAsync(Order order);
}

// Infrastructure implements interface
// Infrastructure/Persistence/OrderRepository.cs
public class OrderRepository : IOrderRepository
{
    private readonly ApplicationDbContext _context;
    
    public OrderRepository(ApplicationDbContext context)
    {
        _context = context;
    }
    
    public async Task<Order?> GetByIdAsync(Guid id)
    {
        return await _context.Orders
            .Include(o => o.Items)
            .FirstOrDefaultAsync(o => o.Id == id);
    }
    
    public async Task AddAsync(Order order)
    {
        await _context.Orders.AddAsync(order);
    }
    
    public async Task UpdateAsync(Order order)
    {
        _context.Orders.Update(order);
    }
}

Key Fixes:

Domain has zero infrastructure dependencies
Repository interface defined in Application layer
Implementation in Infrastructure layer
Can test domain logic in isolation
Can swap database without touching domain

Mistake #2: Anemic Domain Model (AKA Data Bags)

The Problem: Your Entities Are Just Fancy Structs

Ever create an entity that looks like this?

public class Order {
    public Guid Id { get; set; }
    public decimal Total { get; set; }
    public string Status { get; set; }
}

Congratulations, you’ve created a glorified data transfer object. An anemic domain model is just a bag of getters and setters with zero behavior. All your actual business logic? Scattered across service classes.

This defeats the entire point of Domain-Driven Design. Your domain should be rich with behavior, not anemic data containers.

Bad Example: All Logic in Services

// Anemic Domain Entity - Just a data bag
public class BankAccount
{
    public Guid Id { get; set; }
    public decimal Balance { get; set; }  // Public setter
    public bool IsActive { get; set; }    // Public setter
    public DateTime CreatedAt { get; set; }
}

// All business logic in service layer
public class BankAccountService
{
    private readonly IRepository<BankAccount> _repository;
    
    // Business rules scattered in service
    public async Task Deposit(Guid accountId, decimal amount)
    {
        var account = await _repository.GetByIdAsync(accountId);
        
        // Validation logic in service
        if (amount <= 0)
            throw new ArgumentException("Amount must be positive");
            
        if (!account.IsActive)
            throw new InvalidOperationException("Account is not active");
        
        // Business logic in service
        account.Balance += amount;
        
        await _repository.UpdateAsync(account);
    }
    
    // More business rules in service
    public async Task Withdraw(Guid accountId, decimal amount)
    {
        var account = await _repository.GetByIdAsync(accountId);
        
        if (amount <= 0)
            throw new ArgumentException("Amount must be positive");
            
        if (!account.IsActive)
            throw new InvalidOperationException("Account is not active");
            
        // Business rule in service
        if (account.Balance < amount)
            throw new InvalidOperationException("Insufficient funds");
        
        account.Balance -= amount;
        
        await _repository.UpdateAsync(account);
    }
}

Problems:

Business rules scattered across services
Easy to bypass validation
Hard to test business logic
Duplicate validation code
Entity can be in invalid state

Correct Approach: Rich Domain Model

// Rich Domain Entity - Encapsulates behavior
public class BankAccount
{
    public Guid Id { get; private set; }  // Private setters
    public decimal Balance { get; private set; }
    public bool IsActive { get; private set; }
    public DateTime CreatedAt { get; private set; }
    
    private readonly List<Transaction> _transactions = new();
    public IReadOnlyList<Transaction> Transactions => _transactions.AsReadOnly();
    
    private BankAccount() { } // For EF Core
    
    // Factory method with validation
    public static BankAccount Create(Guid customerId, decimal initialDeposit)
    {
        if (initialDeposit < 0)
            throw new ArgumentException("Initial deposit cannot be negative");
            
        var account = new BankAccount
        {
            Id = Guid.NewGuid(),
            Balance = initialDeposit,
            IsActive = true,
            CreatedAt = DateTime.UtcNow
        };
        
        if (initialDeposit > 0)
        {
            account.AddTransaction(TransactionType.Deposit, initialDeposit);
        }
        
        return account;
    }
    
    // Business logic in domain
    public void Deposit(decimal amount)
    {
        if (amount <= 0)
            throw new ArgumentException("Amount must be positive");
            
        EnsureAccountIsActive();
        
        Balance += amount;
        AddTransaction(TransactionType.Deposit, amount);
    }
    
    // Business logic in domain
    public void Withdraw(decimal amount)
    {
        if (amount <= 0)
            throw new ArgumentException("Amount must be positive");
            
        EnsureAccountIsActive();
        
        if (Balance < amount)
            throw new InvalidOperationException(
                $"Insufficient funds. Available: {Balance}, Requested: {amount}"
            );
        
        Balance -= amount;
        AddTransaction(TransactionType.Withdrawal, amount);
    }
    
    public void Deactivate()
    {
        if (Balance > 0)
            throw new InvalidOperationException(
                "Cannot deactivate account with positive balance"
            );
            
        IsActive = false;
    }
    
    public void Activate() => IsActive = true;
    
    // Encapsulated validation
    private void EnsureAccountIsActive()
    {
        if (!IsActive)
            throw new InvalidOperationException("Account is not active");
    }
    
    private void AddTransaction(TransactionType type, decimal amount)
    {
        _transactions.Add(new Transaction
        {
            Id = Guid.NewGuid(),
            Type = type,
            Amount = amount,
            Timestamp = DateTime.UtcNow
        });
    }
    
    // Calculated properties
    public decimal GetTotalDeposits() 
        => _transactions
            .Where(t => t.Type == TransactionType.Deposit)
            .Sum(t => t.Amount);
            
    public decimal GetTotalWithdrawals()
        => _transactions
            .Where(t => t.Type == TransactionType.Withdrawal)
            .Sum(t => t.Amount);
}

// Application service is now thin
public class BankAccountService
{
    private readonly IBankAccountRepository _repository;
    private readonly IUnitOfWork _unitOfWork;
    
    // Service only orchestrates
    public async Task DepositAsync(Guid accountId, decimal amount)
    {
        var account = await _repository.GetByIdAsync(accountId);
        
        if (account == null)
            throw new NotFoundException($"Account {accountId} not found");
        
        // Domain handles all business logic
        account.Deposit(amount);
        
        await _unitOfWork.SaveChangesAsync();
    }
    
    public async Task WithdrawAsync(Guid accountId, decimal amount)
    {
        var account = await _repository.GetByIdAsync(accountId);
        
        if (account == null)
            throw new NotFoundException($"Account {accountId} not found");
        
        // Domain handles all business logic
        account.Withdraw(amount);
        
        await _unitOfWork.SaveChangesAsync();
    }
}

Key Improvements:

Business logic in domain where it belongs
Impossible to bypass validation (private setters)
Self-documenting - methods show what the entity can do
Easy to test - test entity methods directly
No duplicate code - validation in one place
Service layer is thin orchestration

Mistake #3: Exposing Entities Directly Through API

The Problem: The Day Your Domain Leaked Into Production

Picture this: You return your User entity directly from your API. Works great! Ship it.

Three months later, a security audit reveals you’ve been exposing password hashes, internal IDs, and database relationships to anyone who calls your endpoint. Oops.

Or worse: you need to add a field to your domain entity, and suddenly you’ve broken your API contract with every client.

The rule: Never, ever expose domain entities directly through your API. Use DTOs.

Bad Example: Exposing Domain Entities

// Domain Entity
public class User
{
    public Guid Id { get; set; }
    public string Username { get; set; }
    public string PasswordHash { get; set; }  // Sensitive data!
    public string Email { get; set; }
    public List<Order> Orders { get; set; }   // Complex navigation
    public DateTime CreatedAt { get; set; }
    public DateTime? LastLoginAt { get; set; }
}

// Controller exposing entity directly
[ApiController]
[Route("api/[controller]")]
public class UsersController : ControllerBase
{
    private readonly IUserRepository _repository;
    
    // Returns domain entity directly
    [HttpGet("{id}")]
    public async Task<ActionResult<User>> GetUser(Guid id)
    {
        var user = await _repository.GetByIdAsync(id);
        
        // Exposing sensitive data and internal structure
        return Ok(user);
    }
    
    // Accepts domain entity as input
    [HttpPost]
    public async Task<ActionResult<User>> CreateUser([FromBody] User user)
    {
        await _repository.AddAsync(user);
        return CreatedAtAction(nameof(GetUser), new { id = user.Id }, user);
    }
}

Problems:

Security risk: Exposes PasswordHash and other sensitive data
Tight coupling: API contract depends on domain structure
Circular references: Navigation properties cause serialization issues
Over-fetching: Returns more data than needed
Breaking changes: Domain changes break API contracts

Correct Approach: Use DTOs

// Domain Entity (unchanged)
public class User
{
    public Guid Id { get; private set; }
    public string Username { get; private set; }
    public string PasswordHash { get; private set; }
    public string Email { get; private set; }
    public DateTime CreatedAt { get; private set; }
    public DateTime? LastLoginAt { get; private set; }
    
    private readonly List<Order> _orders = new();
    public IReadOnlyList<Order> Orders => _orders.AsReadOnly();
    
    // Domain methods...
}

// Application Layer DTOs
public record UserDto
{
    public Guid Id { get; init; }
    public string Username { get; init; }
    public string Email { get; init; }
    public DateTime CreatedAt { get; init; }
    public int OrderCount { get; init; }
}

public record CreateUserDto
{
    public string Username { get; init; }
    public string Email { get; init; }
    public string Password { get; init; }  // Plain password, will be hashed
}

public record UserDetailDto
{
    public Guid Id { get; init; }
    public string Username { get; init; }
    public string Email { get; init; }
    public DateTime CreatedAt { get; init; }
    public DateTime? LastLoginAt { get; init; }
    public List<OrderSummaryDto> RecentOrders { get; init; }
}

// Application Service with mapping
public class UserService
{
    private readonly IUserRepository _repository;
    private readonly IPasswordHasher _passwordHasher;
    private readonly IUnitOfWork _unitOfWork;
    
    public async Task<UserDto> GetUserAsync(Guid id)
    {
        var user = await _repository.GetByIdAsync(id);
        
        if (user == null)
            throw new NotFoundException($"User {id} not found");
        
        // Map to DTO
        return new UserDto
        {
            Id = user.Id,
            Username = user.Username,
            Email = user.Email,
            CreatedAt = user.CreatedAt,
            OrderCount = user.Orders.Count
        };
    }
    
    public async Task<UserDto> CreateUserAsync(CreateUserDto dto)
    {
        // Hash password before creating entity
        var passwordHash = _passwordHasher.Hash(dto.Password);
        
        // Use domain factory method
        var user = User.Create(dto.Username, dto.Email, passwordHash);
        
        await _repository.AddAsync(user);
        await _unitOfWork.SaveChangesAsync();
        
        // Return DTO, not entity
        return new UserDto
        {
            Id = user.Id,
            Username = user.Username,
            Email = user.Email,
            CreatedAt = user.CreatedAt,
            OrderCount = 0
        };
    }
}

// Controller using DTOs
[ApiController]
[Route("api/[controller]")]
public class UsersController : ControllerBase
{
    private readonly UserService _userService;
    
    [HttpGet("{id}")]
    [ProducesResponseType(typeof(UserDto), 200)]
    [ProducesResponseType(404)]
    public async Task<ActionResult<UserDto>> GetUser(Guid id)
    {
        try
        {
            var user = await _userService.GetUserAsync(id);
            return Ok(user);
        }
        catch (NotFoundException)
        {
            return NotFound();
        }
    }
    
    [HttpPost]
    [ProducesResponseType(typeof(UserDto), 201)]
    [ProducesResponseType(400)]
    public async Task<ActionResult<UserDto>> CreateUser(
        [FromBody] CreateUserDto dto)
    {
        var user = await _userService.CreateUserAsync(dto);
        return CreatedAtAction(nameof(GetUser), new { id = user.Id }, user);
    }
}

Key Improvements:

Security: Sensitive data never exposed
Flexibility: Can shape API response independently
Versioning: Can maintain multiple DTO versions
Performance: Return only needed data
Stability: Domain changes don’t break API

Mistake #4: Primitive Obsession (Ignoring Value Objects)

The Problem: Everything Is a String or Decimal

Raise your hand if you’ve ever written a method like this:

public void UpdatePrice(decimal price, string currency) { }

Now imagine calling it:

UpdatePrice(100, "USD"); // Correct
UpdatePrice("USD", 100); // 💥 Compiles but wrong!

The compiler can’t save you here because everything’s just primitives. And where do you validate that currency code? In every method that uses it?

This is primitive obsession, and Value Objects are the cure.

Bad Example: Primitive Obsession

// Using primitives everywhere
public class Product
{
    public Guid Id { get; set; }
    public string Name { get; set; }
    public decimal Price { get; set; }        // Just a decimal
    public string Currency { get; set; }      // Separate string
    public string Email { get; set; }         // Just a string
    public string Phone { get; set; }         // Just a string
}

// Validation scattered everywhere
public class ProductService
{
    public async Task CreateProduct(string name, decimal price, string currency)
    {
        // Validation repeated in multiple places
        if (price < 0)
            throw new ArgumentException("Price cannot be negative");
            
        if (string.IsNullOrWhiteSpace(currency))
            throw new ArgumentException("Currency is required");
            
        if (currency.Length != 3)
            throw new ArgumentException("Currency must be 3 characters");
        
        // Easy to mix up price and currency
        var product = new Product
        {
            Price = price,
            Currency = currency.ToUpperInvariant()
        };
    }
    
    public decimal CalculateTotal(decimal price1, string curr1, 
                                   decimal price2, string curr2)
    {
        // Easy to make mistakes
        if (curr1 != curr2)
            throw new InvalidOperationException("Different currencies");
            
        return price1 + price2;
    }
}

Problems:

Validation duplicated everywhere
Easy to pass wrong parameters (decimal, string - which is which?)
No encapsulation of business rules
Can create invalid combinations

Correct Approach: Use Value Objects

// Money Value Object
public sealed class Money : ValueObject
{
    public decimal Amount { get; private set; }
    public Currency Currency { get; private set; }
    
    private Money(decimal amount, Currency currency)
    {
        Amount = amount;
        Currency = currency;
    }
    
    // Factory method with validation
    public static Money Create(decimal amount, Currency currency)
    {
        if (amount < 0)
            throw new ArgumentException("Amount cannot be negative");
            
        return new Money(amount, currency);
    }
    
    // Domain operations
    public Money Add(Money other)
    {
        if (Currency != other.Currency)
            throw new InvalidOperationException(
                $"Cannot add {other.Currency} to {Currency}"
            );
            
        return new Money(Amount + other.Amount, Currency);
    }
    
    public Money Multiply(decimal multiplier)
    {
        return new Money(Amount * multiplier, Currency);
    }
    
    // Value equality
    protected override IEnumerable<object> GetEqualityComponents()
    {
        yield return Amount;
        yield return Currency;
    }
    
    public override string ToString() => $"{Amount:N2} {Currency}";
}

// Currency Value Object (could also be an enum)
public sealed class Currency : ValueObject
{
    public string Code { get; private set; }
    
    private Currency(string code)
    {
        Code = code;
    }
    
    public static Currency USD => new("USD");
    public static Currency EUR => new("EUR");
    public static Currency GBP => new("GBP");
    
    public static Currency FromCode(string code)
    {
        if (string.IsNullOrWhiteSpace(code))
            throw new ArgumentException("Currency code required");
            
        if (code.Length != 3)
            throw new ArgumentException("Currency code must be 3 characters");
            
        return new Currency(code.ToUpperInvariant());
    }
    
    protected override IEnumerable<object> GetEqualityComponents()
    {
        yield return Code;
    }
    
    public override string ToString() => Code;
}

// Email Value Object
public sealed class Email : ValueObject
{
    public string Value { get; private set; }
    
    private Email(string value)
    {
        Value = value;
    }
    
    public static Email Create(string email)
    {
        if (string.IsNullOrWhiteSpace(email))
            throw new ArgumentException("Email is required");
            
        // Validation in one place
        if (!IsValidEmail(email))
            throw new ArgumentException($"Invalid email: {email}");
            
        return new Email(email.ToLowerInvariant());
    }
    
    private static bool IsValidEmail(string email)
    {
        try
        {
            var addr = new System.Net.Mail.MailAddress(email);
            return addr.Address == email;
        }
        catch
        {
            return false;
        }
    }
    
    protected override IEnumerable<object> GetEqualityComponents()
    {
        yield return Value;
    }
    
    public override string ToString() => Value;
}

// Product using Value Objects
public class Product
{
    public Guid Id { get; private set; }
    public string Name { get; private set; }
    public Money Price { get; private set; }    // Type-safe
    public Email ContactEmail { get; private set; }
    
    public static Product Create(string name, Money price, Email email)
    {
        if (string.IsNullOrWhiteSpace(name))
            throw new ArgumentException("Name is required");
            
        return new Product
        {
            Id = Guid.NewGuid(),
            Name = name,
            Price = price,  // Already validated
            ContactEmail = email  // Already validated
        };
    }
    
    public void UpdatePrice(Money newPrice)
    {
        // Type-safe - can't accidentally pass wrong type
        Price = newPrice;
    }
    
    public Money CalculateDiscountedPrice(decimal discountPercent)
    {
        // Rich operations on value objects
        var discountMultiplier = (100 - discountPercent) / 100;
        return Price.Multiply(discountMultiplier);
    }
}

// Service is now cleaner
public class ProductService
{
    public async Task<ProductDto> CreateProduct(
        string name, 
        decimal priceAmount, 
        string currencyCode,
        string emailAddress)
    {
        // Create value objects (validation happens here)
        var currency = Currency.FromCode(currencyCode);
        var price = Money.Create(priceAmount, currency);
        var email = Email.Create(emailAddress);
        
        // Create product with validated value objects
        var product = Product.Create(name, price, email);
        
        await _repository.AddAsync(product);
        await _unitOfWork.SaveChangesAsync();
        
        return MapToDto(product);
    }
}

Key Benefits:

Single validation point - validated in value object constructor
Type safety - can’t mix up parameters
Encapsulation - behavior with data
Testability - easy to test value objects in isolation
Reusability - use Money everywhere in domain
Expressiveness - Money is more meaningful than decimal

Mistake #5: Repository Anti-patterns (The IQueryable Trap)

The Problem: Your “Repository” Is Just EF Core with Extra Steps

You’ve seen this pattern everywhere:

public interface IRepository<T> {
    IQueryable<T> GetAll();
}

Looks clean, right? One interface for all entities!

Except now your application layer is writing LINQ queries with .Include() and .ThenInclude(). You’ve just leaked Entity Framework Core into your business logic. Your “abstraction” isn’t abstracting anything.

When you try to switch databases or write unit tests, you’ll feel the pain.

Bad Example: Generic Repository with IQueryable

// Generic repository exposing IQueryable
public interface IRepository<T> where T : class
{
    IQueryable<T> GetAll();  // Exposes IQueryable
    IQueryable<T> Find(Expression<Func<T, bool>> predicate);  // LINQ in app layer
    Task<T> GetByIdAsync(Guid id);
    Task AddAsync(T entity);
    Task UpdateAsync(T entity);
    Task DeleteAsync(T entity);
}

// Application layer doing database queries
public class OrderService
{
    private readonly IRepository<Order> _repository;
    
    public async Task<List<OrderDto>> GetCustomerOrders(Guid customerId)
    {
        // Writing LINQ queries in service layer
        var orders = await _repository
            .GetAll()
            .Where(o => o.CustomerId == customerId)
            .Include(o => o.Items)  // EF Core specific!
            .ThenInclude(i => i.Product)
            .OrderByDescending(o => o.CreatedAt)
            .Take(10)
            .ToListAsync();
            
        return orders.Select(MapToDto).ToList();
    }
    
    public async Task<decimal> GetTotalRevenue(DateTime from, DateTime to)
    {
        // Complex query logic in service
        return await _repository
            .GetAll()
            .Where(o => o.CreatedAt >= from && o.CreatedAt <= to)
            .Where(o => o.Status == OrderStatus.Completed)
            .SelectMany(o => o.Items)
            .SumAsync(i => i.Price * i.Quantity);
    }
}

Problems:

Leaking abstraction: Application knows about Include, ThenInclude
Tight coupling: Coupled to Entity Framework
Hard to test: Can’t easily mock IQueryable
Performance issues: Easy to create N+1 queries
Breaking encapsulation: Data access logic scattered

Correct Approach: Specific Repository Methods

// Specific repository interface with meaningful methods
public interface IOrderRepository
{
    Task<Order?> GetByIdAsync(Guid id);
    Task<Order?> GetByIdWithItemsAsync(Guid id);  // Explicit include
    Task<IReadOnlyList<Order>> GetCustomerOrdersAsync(
        Guid customerId, 
        int pageSize = 10
    );
    Task<IReadOnlyList<Order>> GetOrdersByStatusAsync(OrderStatus status);
    Task<IReadOnlyList<Order>> GetOrdersInDateRangeAsync(
        DateTime from, 
        DateTime to
    );
    Task<decimal> GetTotalRevenueAsync(DateTime from, DateTime to);
    Task<bool> ExistsAsync(Guid id);
    Task AddAsync(Order order);
    Task UpdateAsync(Order order);
    Task DeleteAsync(Order order);
}

// Repository implementation with optimized queries
public class OrderRepository : IOrderRepository
{
    private readonly ApplicationDbContext _context;
    
    public OrderRepository(ApplicationDbContext context)
    {
        _context = context;
    }
    
    public async Task<Order?> GetByIdAsync(Guid id)
    {
        return await _context.Orders
            .FirstOrDefaultAsync(o => o.Id == id);
    }
    
    // Explicit method for including related data
    public async Task<Order?> GetByIdWithItemsAsync(Guid id)
    {
        return await _context.Orders
            .Include(o => o.Items)
                .ThenInclude(i => i.Product)
            .FirstOrDefaultAsync(o => o.Id == id);
    }
    
    // Domain-specific query encapsulated
    public async Task<IReadOnlyList<Order>> GetCustomerOrdersAsync(
        Guid customerId, 
        int pageSize = 10)
    {
        return await _context.Orders
            .Where(o => o.CustomerId == customerId)
            .Include(o => o.Items)
            .OrderByDescending(o => o.CreatedAt)
            .Take(pageSize)
            .ToListAsync();
    }
    
    // Complex business query in repository
    public async Task<decimal> GetTotalRevenueAsync(
        DateTime from, 
        DateTime to)
    {
        return await _context.Orders
            .Where(o => o.CreatedAt >= from && o.CreatedAt <= to)
            .Where(o => o.Status == OrderStatus.Completed)
            .SelectMany(o => o.Items)
            .SumAsync(i => i.TotalPrice);
    }
    
    public async Task AddAsync(Order order)
    {
        await _context.Orders.AddAsync(order);
    }
    
    public async Task UpdateAsync(Order order)
    {
        _context.Orders.Update(order);
    }
}

// Application service is now cleaner
public class OrderService
{
    private readonly IOrderRepository _repository;
    private readonly IUnitOfWork _unitOfWork;
    
    // Simple method call, no query logic
    public async Task<List<OrderDto>> GetCustomerOrdersAsync(Guid customerId)
    {
        var orders = await _repository.GetCustomerOrdersAsync(customerId);
        return orders.Select(MapToDto).ToList();
    }
    
    // Repository handles the complexity
    public async Task<decimal> GetTotalRevenueAsync(DateTime from, DateTime to)
    {
        return await _repository.GetTotalRevenueAsync(from, to);
    }
    
    // Business logic, not data access logic
    public async Task CompleteOrderAsync(Guid orderId)
    {
        var order = await _repository.GetByIdWithItemsAsync(orderId);
        
        if (order == null)
            throw new NotFoundException($"Order {orderId} not found");
        
        // Domain method handles business rules
        order.Complete();
        
        await _unitOfWork.SaveChangesAsync();
    }
}

Key Improvements:

Encapsulation: Query logic hidden in repository
Testability: Easy to mock specific methods
Performance: Optimized queries in one place
Abstraction: Application doesn’t know about EF Core
Maintainability: Change query logic in one place

Mistake #6: Fat Controllers (The “Just This Once” Syndrome)

The Problem: Your Controller Is 500 Lines Long

“I’ll just put this one little validation check in the controller…”

Famous last words. Three months later, your controller is 500 lines of database queries, validation logic, calculations, and email sending.

Now try writing a unit test for that business logic. Oh wait, you can’t - it’s buried in HTTP infrastructure.

Controllers should be dumb. They handle HTTP requests, call a service, and return HTTP responses. That’s it.

Bad Example: Fat Controllers

// Controller with business logic
[ApiController]
[Route("api/[controller]")]
public class OrdersController : ControllerBase
{
    private readonly ApplicationDbContext _context;
    private readonly IEmailService _emailService;
    
    [HttpPost]
    public async Task<IActionResult> CreateOrder([FromBody] CreateOrderRequest request)
    {
        // Validation in controller
        if (string.IsNullOrWhiteSpace(request.CustomerName))
            return BadRequest("Customer name is required");
            
        if (request.Items == null || !request.Items.Any())
            return BadRequest("Order must have at least one item");
        
        // Business logic in controller
        var customer = await _context.Customers
            .FirstOrDefaultAsync(c => c.Id == request.CustomerId);
            
        if (customer == null)
            return NotFound("Customer not found");
        
        // Database access directly in controller
        var order = new Order
        {
            Id = Guid.NewGuid(),
            CustomerId = request.CustomerId,
            CreatedAt = DateTime.UtcNow,
            Status = "Pending"
        };
        
        // More business logic
        decimal total = 0;
        foreach (var item in request.Items)
        {
            var product = await _context.Products.FindAsync(item.ProductId);
            
            if (product == null)
                return BadRequest($"Product {item.ProductId} not found");
                
            if (product.Stock < item.Quantity)
                return BadRequest($"Insufficient stock for {product.Name}");
            
            total += product.Price * item.Quantity;
            product.Stock -= item.Quantity;
            
            order.Items.Add(new OrderItem
            {
                ProductId = item.ProductId,
                Quantity = item.Quantity,
                Price = product.Price
            });
        }
        
        order.TotalAmount = total;
        
        // More database operations
        _context.Orders.Add(order);
        await _context.SaveChangesAsync();
        
        // Additional business logic (email)
        await _emailService.SendOrderConfirmationAsync(customer.Email, order);
        
        return CreatedAtAction(nameof(GetOrder), new { id = order.Id }, order);
    }
}

Problems:

Untestable: Hard to test without HTTP infrastructure
Duplication: Logic repeated in other endpoints
SRP violation: Controller does too much
Hard to reuse: Logic tied to HTTP
Difficult to maintain: Business rules scattered

Correct Approach: Thin Controllers

// Application Service handles business logic
public class OrderService
{
    private readonly IOrderRepository _orderRepository;
    private readonly IProductRepository _productRepository;
    private readonly ICustomerRepository _customerRepository;
    private readonly IEmailService _emailService;
    private readonly IUnitOfWork _unitOfWork;
    private readonly ILogger<OrderService> _logger;
    
    public async Task<OrderDto> CreateOrderAsync(CreateOrderCommand command)
    {
        // Business validation
        var customer = await _customerRepository.GetByIdAsync(command.CustomerId);
        if (customer == null)
            throw new NotFoundException($"Customer {command.CustomerId} not found");
        
        // Use domain factory method
        var order = Order.Create(customer.Id, customer.Name);
        
        // Business logic in domain or service
        foreach (var item in command.Items)
        {
            var product = await _productRepository.GetByIdAsync(item.ProductId);
            
            if (product == null)
                throw new NotFoundException($"Product {item.ProductId} not found");
            
            // Domain method handles business rules
            order.AddItem(product, item.Quantity);
            
            // Domain method handles stock reduction
            product.RemoveStock(item.Quantity);
        }
        
        // Persist changes
        await _orderRepository.AddAsync(order);
        await _unitOfWork.SaveChangesAsync();
        
        // Send notification
        try
        {
            await _emailService.SendOrderConfirmationAsync(customer.Email, order.Id);
        }
        catch (Exception ex)
        {
            _logger.LogWarning(ex, "Failed to send order confirmation email");
            // Don't fail the order creation if email fails
        }
        
        return MapToDto(order);
    }
}

// Thin controller
[ApiController]
[Route("api/[controller]")]
public class OrdersController : ControllerBase
{
    private readonly OrderService _orderService;
    private readonly ILogger<OrdersController> _logger;
    
    public OrdersController(
        OrderService orderService,
        ILogger<OrdersController> logger)
    {
        _orderService = orderService;
        _logger = logger;
    }
    
    /// <summary>
    /// Create a new order
    /// </summary>
    [HttpPost]
    [ProducesResponseType(typeof(OrderDto), 201)]
    [ProducesResponseType(400)]
    [ProducesResponseType(404)]
    public async Task<ActionResult<OrderDto>> CreateOrder(
        [FromBody] CreateOrderCommand command)
    {
        try
        {
            // Controller only orchestrates
            var order = await _orderService.CreateOrderAsync(command);
            
            return CreatedAtAction(
                nameof(GetOrder), 
                new { id = order.Id }, 
                order
            );
        }
        catch (NotFoundException ex)
        {
            _logger.LogWarning(ex, "Resource not found");
            return NotFound(ex.Message);
        }
        catch (InvalidOperationException ex)
        {
            _logger.LogWarning(ex, "Business rule violation");
            return BadRequest(ex.Message);
        }
        catch (ArgumentException ex)
        {
            _logger.LogWarning(ex, "Invalid argument");
            return BadRequest(ex.Message);
        }
        catch (Exception ex)
        {
            _logger.LogError(ex, "Unexpected error creating order");
            return StatusCode(500, "An error occurred while creating the order");
        }
    }
    
    [HttpGet("{id}")]
    [ProducesResponseType(typeof(OrderDto), 200)]
    [ProducesResponseType(404)]
    public async Task<ActionResult<OrderDto>> GetOrder(Guid id)
    {
        try
        {
            var order = await _orderService.GetOrderAsync(id);
            return Ok(order);
        }
        catch (NotFoundException)
        {
            return NotFound();
        }
    }
}

Key Improvements:

Separation: Business logic in service, HTTP in controller
Testability: Can test service without HTTP
Reusability: Service can be called from anywhere
Maintainability: Business rules in one place
SRP: Controller handles HTTP, service handles business

Mistake #7: Over-Engineering Everything

The Problem: 12 Files for a Lookup Table

I once worked on a project where someone created:

A domain entity for Country
An interface ICountryRepository
An implementation CountryRepository
A service CountryService
A DTO CountryDto
A mapper CountryMapper
A controller CountriesController

…for a simple lookup table that just lists countries. No business logic. No validation. Just read-only data.

Clean Architecture doesn’t mean every feature needs 12 files. Sometimes, a simple query in a controller is fine.

Bad Example: Over-Engineered Simple Feature

// Over-engineered for a simple lookup table

// Domain Layer - Overkill for simple data
public class Country
{
    public Guid Id { get; private set; }
    public string Code { get; private set; }
    public string Name { get; private set; }
    
    private Country() { }
    
    public static Country Create(string code, string name)
    {
        if (string.IsNullOrWhiteSpace(code))
            throw new ArgumentException("Code required");
        // ... validation
        return new Country { Id = Guid.NewGuid(), Code = code, Name = name };
    }
}

// Application Layer - Unnecessary for CRUD
public interface ICountryRepository
{
    Task<Country?> GetByIdAsync(Guid id);
    Task<IReadOnlyList<Country>> GetAllAsync();
    Task AddAsync(Country country);
}

public class CountryService
{
    private readonly ICountryRepository _repository;
    
    public async Task<List<CountryDto>> GetAllCountriesAsync()
    {
        var countries = await _repository.GetAllAsync();
        return countries.Select(c => new CountryDto
        {
            Id = c.Id,
            Code = c.Code,
            Name = c.Name
        }).ToList();
    }
}

// Presentation Layer
[ApiController]
[Route("api/[controller]")]
public class CountriesController : ControllerBase
{
    private readonly CountryService _service;
    
    [HttpGet]
    public async Task<ActionResult<List<CountryDto>>> GetAll()
    {
        return Ok(await _service.GetAllCountriesAsync());
    }
}

// Result: 5+ files for a simple lookup table!

Correct Approach: Keep It Simple

// Simple approach for simple data

// 1. Use a simple model (not a complex entity)
public class Country
{
    public int Id { get; set; }
    public string Code { get; set; }
    public string Name { get; set; }
}

// 2. Direct query in controller (for simple read-only data)
[ApiController]
[Route("api/[controller]")]
public class CountriesController : ControllerBase
{
    private readonly ApplicationDbContext _context;
    
    [HttpGet]
    [ResponseCache(Duration = 3600)] // Cache for 1 hour
    public async Task<ActionResult<List<Country>>> GetAll()
    {
        // Direct query for simple read-only data
        return await _context.Countries
            .OrderBy(c => c.Name)
            .ToListAsync();
    }
}

// Total: 1 file. Simple. Clear. Maintainable.

When to Keep It Simple:

Lookup tables (countries, states, categories)
Read-only data
No business rules
No complex validation
Simple CRUD operations

When to Use Full Architecture:

Complex business logic
Multiple validation rules
Domain events needed
Complex aggregates
Frequent changes expected

Summary: Dos and Don’ts

DO

Practice	Benefit
Keep domain pure (no infrastructure)	Testable, flexible
Use rich domain models with behavior	Encapsulation, less bugs
Create specific repository methods	Clear intent, optimized
Use DTOs for API contracts	Decoupling, security
Implement value objects	Type safety, validation
Keep controllers thin	Testability, reusability
Use factory methods	Enforce invariants
Apply patterns pragmatically	Right tool for the job

DON’T

Anti-pattern	Problem
Reference outer layers from inner	Breaks dependency rule
Create anemic domain models	Business logic scattered
Expose IQueryable from repositories	Leaky abstraction
Return entities from API	Tight coupling, security
Use primitives for domain concepts	Validation duplication
Put business logic in controllers	Hard to test, duplicate code
Over-engineer simple features	Unnecessary complexity
Generic repository for everything	One size doesn’t fit all

Checklist: Is Your Clean Architecture Clean?

Use this checklist to evaluate your implementation:

Domain Layer

Zero dependencies on outer layers
No reference to Entity Framework, ASP.NET, etc.
Entities have private setters
Factory methods for object creation
Business rules enforced in domain
Value objects for domain concepts
Domain events for important changes

Application Layer

Defines repository interfaces
Contains use case logic
Uses DTOs for data transfer
No direct database access
Thin orchestration services
Clear command/query separation

Infrastructure Layer

Implements interfaces from Application
Contains EF Core configurations
Repository implementations optimized
No business logic here

Presentation Layer

Controllers are thin (< 30 lines per action)
Only HTTP concerns handled
Exception handling to HTTP status codes
No database access
No business logic

Action Plan: Fixing Your Architecture

If you identified issues in your codebase, follow this refactoring plan:

Phase 1: Quick Wins (Week 1)

Add private setters to domain entities
Create DTOs for API responses
Move validation into domain entities
Extract factory methods for entity creation

Phase 2: Repository Refactoring (Week 2-3)

Remove IQueryable from repository interfaces
Create specific methods for common queries
Encapsulate complex queries in repositories
Add Unit of Work for transaction management

Phase 3: Rich Domain Model (Week 4-6)

Move business logic from services to entities
Create value objects for domain concepts
Implement domain events
Add domain factory methods

Phase 4: Clean Boundaries (Week 7-8)

Remove infrastructure dependencies from domain
Create clear interfaces in Application layer
Refactor controllers to be thin
Implement proper layering

What I’ve Learned (The Hard Way)

I’ve made every one of these mistakes. Some multiple times. Here’s what finally clicked for me:

Clean Architecture isn’t about following rules perfectly - it’s about protecting your business logic from changing technology.

The 7 mistakes we covered:

Breaking the Dependency Rule → Your domain should compile without ANY infrastructure references
Anemic Domain Models → If your entity is all getters/setters, you’re doing it wrong
Exposing Entities → Use DTOs or prepare for security issues and breaking changes
Ignoring Value Objects → Stop using primitives for domain concepts with rules
Repository Anti-patterns → If you’re exposing IQueryable, you haven’t abstracted anything
Fat Controllers → Controllers orchestrate, they don’t implement
Over-engineering → Not everything needs full Clean Architecture treatment

The One Question That Matters

Whenever you’re unsure, ask yourself:

“Can I test this business rule without spinning up a database, HTTP server, or any other infrastructure?”

If no, you’ve coupled your business logic to something that will change. Fix it now, or pay later.

Start Here

Pick ONE mistake from this list that you’re making right now. Just one. Fix it in your current project.

Next week, fix another one.

Clean Architecture is a journey, not a destination. Don’t try to fix everything at once - you’ll burn out or give up.

Remember: The best architecture is the simplest one that solves your problem. Start simple, add complexity only when pain points emerge.

Common Clean Architecture Mistakes in .NET (And How to Fix Them)

Why Even Good Developers Get This Wrong

Mistake #1: Breaking the Dependency Rule

The Problem: “Just One Little Using Statement…”

Bad Example: Domain Depending on Infrastructure (Don’t Do This)

Correct Approach: Pure Domain with Repository Abstraction

Mistake #2: Anemic Domain Model (AKA Data Bags)

The Problem: Your Entities Are Just Fancy Structs

Bad Example: All Logic in Services

Correct Approach: Rich Domain Model

Mistake #3: Exposing Entities Directly Through API

The Problem: The Day Your Domain Leaked Into Production

Bad Example: Exposing Domain Entities

Correct Approach: Use DTOs

Mistake #4: Primitive Obsession (Ignoring Value Objects)

The Problem: Everything Is a String or Decimal

Bad Example: Primitive Obsession

Correct Approach: Use Value Objects

Mistake #5: Repository Anti-patterns (The IQueryable Trap)

The Problem: Your “Repository” Is Just EF Core with Extra Steps

Bad Example: Generic Repository with IQueryable

Correct Approach: Specific Repository Methods

Mistake #6: Fat Controllers (The “Just This Once” Syndrome)

The Problem: Your Controller Is 500 Lines Long

Bad Example: Fat Controllers

Correct Approach: Thin Controllers

Mistake #7: Over-Engineering Everything

The Problem: 12 Files for a Lookup Table

Bad Example: Over-Engineered Simple Feature

Correct Approach: Keep It Simple

Summary: Dos and Don’ts

DO

DON’T

Checklist: Is Your Clean Architecture Clean?

Domain Layer

Application Layer

Infrastructure Layer

Presentation Layer

Action Plan: Fixing Your Architecture

Phase 1: Quick Wins (Week 1)

Phase 2: Repository Refactoring (Week 2-3)

Phase 3: Rich Domain Model (Week 4-6)

Phase 4: Clean Boundaries (Week 7-8)

What I’ve Learned (The Hard Way)

The One Question That Matters

Start Here

Further Reading

Related Content