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

December 22, 2025 · 4768 words · 21 min read

Contents

Clean Architecture .NET Software Architecture Best Practices Antipatterns

Clean Architecture promises maintainability, testability, and flexibility. But without careful implementation, it can become a nightmare of over-engineering, broken abstractions, and violated principles. Let’s explore the most common mistakes and learn how to avoid them.

📚 New to Clean Architecture? Check out our comprehensive guides:
Clean Architecture in .NET Overall
Clean Architecture Real-World Example

🎯 Introduction: Why Developers Struggle

Clean Architecture is powerful but easy to misuse. Common issues include:

❌ Breaking the Dependency Rule
❌ Creating anemic domain models
❌ Over-engineering simple features
❌ Leaking infrastructure concerns into domain
❌ Misusing repositories and DTOs

Let’s dive into each mistake with real-world examples and solutions.

🚫 Mistake #1: Breaking the Dependency Rule

The Problem

The most fundamental rule of Clean Architecture is: Dependencies point inward. Inner layers should NEVER depend on outer layers.

❌ Bad Example: Domain Depending on Infrastructure

// ❌ 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

The Problem

An anemic domain model is one that contains only data (properties) with no behavior (methods). All business logic ends up in services, defeating the purpose of Clean Architecture.

❌ 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

Returning domain entities directly from controllers creates tight coupling between internal domain structure and external API contracts.

❌ 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: Ignoring Value Objects

The Problem

Using primitives for domain concepts that have validation rules and behavior leads to scattered validation and potential bugs.

❌ 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 Problem

Repositories that are too generic or expose IQueryable leak persistence concerns into the application layer.

❌ 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: Putting Business Logic in Controllers

The Problem

Controllers should only handle HTTP concerns. Business logic in controllers makes testing difficult and leads to code duplication.

❌ 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 Simple Features

The Problem

Applying Clean Architecture to every feature, no matter how simple, creates unnecessary complexity.

❌ 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

🎓 Conclusion

Clean Architecture is powerful but easy to misuse. The most common mistakes are:

Breaking the Dependency Rule - Keep domain pure
Anemic Domain Models - Put behavior with data
Exposing Entities - Use DTOs for external contracts
Ignoring Value Objects - Encapsulate domain concepts
Repository Anti-patterns - Be specific, hide implementation
Fat Controllers - Keep business logic in domain/services
Over-engineering - Apply patterns pragmatically

Remember: Clean Architecture is about separating concerns and protecting your business logic. When in doubt, ask yourself:

“Can I test this business rule without any infrastructure?”

If the answer is no, you might be making one of these mistakes.

Key Principle

The best architecture is the simplest one that solves your problem. Start simple, add complexity only when needed.

📚 Further Reading

Have you made any of these mistakes? Share your experiences in the comments below! 💬

Found this helpful? Share it with your team! 🚀

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

🎯 Introduction: Why Developers Struggle

🚫 Mistake #1: Breaking the Dependency Rule

The Problem

❌ Bad Example: Domain Depending on Infrastructure

✅ Correct Approach: Pure Domain with Repository Abstraction

🚫 Mistake #2: Anemic Domain Model

The Problem

❌ Bad Example: All Logic in Services

✅ Correct Approach: Rich Domain Model

🚫 Mistake #3: Exposing Entities Directly Through API

The Problem

❌ Bad Example: Exposing Domain Entities

✅ Correct Approach: Use DTOs

🚫 Mistake #4: Ignoring Value Objects

The Problem

❌ Bad Example: Primitive Obsession

✅ Correct Approach: Use Value Objects

🚫 Mistake #5: Repository Anti-patterns

The Problem

❌ Bad Example: Generic Repository with IQueryable

✅ Correct Approach: Specific Repository Methods

🚫 Mistake #6: Putting Business Logic in Controllers

The Problem

❌ Bad Example: Fat Controllers

✅ Correct Approach: Thin Controllers

🚫 Mistake #7: Over-Engineering Simple Features

The Problem

❌ 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)

🎓 Conclusion

Key Principle

📚 Further Reading

Related Content