dotnet-solid-principles
dotnet-solid-principles
Foundational design principles for .NET applications. Covers each SOLID principle with concrete C# anti-patterns and fixes, plus DRY guidance with nuance on when duplication is acceptable. These principles guide class design, interface contracts, and dependency management across all .NET project types.
Scope
- SOLID principles with C# anti-patterns and fixes
- DRY guidance and when duplication is acceptable
- SRP compliance tests and class design heuristics
- Interface segregation and dependency inversion patterns
Out of scope
- Architectural patterns (vertical slices, request pipelines, caching) -- see [skill:dotnet-architecture-patterns]
- DI container mechanics (registration, lifetimes, keyed services) -- see [skill:dotnet-csharp-dependency-injection]
- Code smells and anti-pattern detection -- see [skill:dotnet-csharp-code-smells]
Cross-references: [skill:dotnet-architecture-patterns] for clean architecture and vertical slices, [skill:dotnet-csharp-dependency-injection] for DI registration patterns and lifetime management, [skill:dotnet-csharp-code-smells] for anti-pattern detection, [skill:dotnet-csharp-coding-standards] for naming and style conventions.
Single Responsibility Principle (SRP)
A class should have only one reason to change. Apply the "describe in one sentence" test: if you cannot describe what a class does in one sentence without using "and" or "or", it likely violates SRP.
Anti-Pattern: God Class
// WRONG -- OrderService handles validation, persistence, email, and PDF generation
public class OrderService
{
private readonly AppDbContext _db;
private readonly SmtpClient _smtp;
public OrderService(AppDbContext db, SmtpClient smtp)
{
_db = db;
_smtp = smtp;
}
public async Task<Order> CreateOrderAsync(CreateOrderRequest request)
{
// Validation logic (reason to change #1)
if (string.IsNullOrEmpty(request.CustomerId))
throw new ArgumentException("Customer required");
// Persistence logic (reason to change #2)
var order = new Order { CustomerId = request.CustomerId };
_db.Orders.Add(order);
await _db.SaveChangesAsync();
// Email notification (reason to change #3)
var message = new MailMessage("noreply@shop.com", request.Email,
"Order Confirmed", $"Order {order.Id} created.");
await _smtp.SendMailAsync(message);
// PDF generation (reason to change #4)
GenerateInvoicePdf(order);
return order;
}
private void GenerateInvoicePdf(Order order) { /* ... */ }
}
Fix: Separate Responsibilities
// Each class has one reason to change
public sealed class OrderCreator(
IOrderValidator validator,
IOrderRepository repository,
IOrderNotifier notifier)
{
public async Task<Order> CreateAsync(
CreateOrderRequest request, CancellationToken ct)
{
validator.Validate(request);
var order = await repository.AddAsync(request, ct);
await notifier.OrderCreatedAsync(order, ct);
return order;
}
}
public sealed class OrderValidator : IOrderValidator
{
public void Validate(CreateOrderRequest request)
{
ArgumentException.ThrowIfNullOrEmpty(request.CustomerId);
// ... validation rules
}
}
public sealed class OrderRepository(AppDbContext db) : IOrderRepository
{
public async Task<Order> AddAsync(
CreateOrderRequest request, CancellationToken ct)
{
var order = new Order { CustomerId = request.CustomerId };
db.Orders.Add(order);
await db.SaveChangesAsync(ct);
return order;
}
}
Anti-Pattern: Fat Controller
// WRONG -- controller contains business logic, mapping, and persistence
app.MapPost("/api/orders", async (
CreateOrderRequest request,
AppDbContext db,
ILogger<Program> logger) =>
{
// Validation in the endpoint
if (request.Lines.Count == 0)
return Results.BadRequest("At least one line required");
// Business logic in the endpoint
var total = request.Lines.Sum(l => l.Quantity * l.Price);
if (total > 100_000)
return Results.BadRequest("Order exceeds credit limit");
// Mapping in the endpoint
var order = new Order
{
CustomerId = request.CustomerId,
Total = total,
Lines = request.Lines.Select(l => new OrderLine
{
ProductId = l.ProductId,
Quantity = l.Quantity,
Price = l.Price
}).ToList()
};
// Persistence in the endpoint
db.Orders.Add(order);
await db.SaveChangesAsync();
logger.LogInformation("Order {OrderId} created", order.Id);
return Results.Created($"/api/orders/{order.Id}", order);
});
Move business logic to a handler; keep the endpoint thin:
app.MapPost("/api/orders", async (
CreateOrderRequest request,
IOrderHandler handler,
CancellationToken ct) =>
{
var result = await handler.CreateAsync(request, ct);
return result switch
{
{ IsSuccess: true } => Results.Created(
$"/api/orders/{result.Value.Id}", result.Value),
_ => Results.ValidationProblem(result.Errors)
};
});
Open/Closed Principle (OCP)
Classes should be open for extension but closed for modification. Add new behavior by implementing new types, not by editing existing switch/if chains.
Anti-Pattern: Switch on Type
// WRONG -- adding a new discount type requires modifying this method
public decimal CalculateDiscount(Order order)
{
switch (order.DiscountType)
{
case "Percentage":
return order.Total * order.DiscountValue / 100;
case "FixedAmount":
return order.DiscountValue;
case "BuyOneGetOneFree":
return order.Lines
.Where(l => l.Quantity >= 2)
.Sum(l => l.Price);
default:
return 0;
}
}
Fix: Strategy Pattern
public interface IDiscountStrategy
{
decimal Calculate(Order order);
}
public sealed class PercentageDiscount(decimal percentage) : IDiscountStrategy
{
public decimal Calculate(Order order) =>
order.Total * percentage / 100;
}
public sealed class FixedAmountDiscount(decimal amount) : IDiscountStrategy
{
public decimal Calculate(Order order) =>
Math.Min(amount, order.Total);
}
// New discount type -- no existing code modified
public sealed class BuyOneGetOneFreeDiscount : IDiscountStrategy
{
public decimal Calculate(Order order) =>
order.Lines
.Where(l => l.Quantity >= 2)
.Sum(l => l.Price);
}
// Usage -- resolved via DI or factory
public sealed class OrderPricing(
IEnumerable<IDiscountStrategy> strategies)
{
public decimal ApplyBestDiscount(Order order) =>
strategies.Max(s => s.Calculate(order));
}
Extension via Abstract Classes
When strategies share significant behavior, use an abstract base class:
public abstract class NotificationSender
{
public async Task SendAsync(Notification notification, CancellationToken ct)
{
// Shared behavior: validation and logging
ArgumentNullException.ThrowIfNull(notification);
await SendCoreAsync(notification, ct);
}
protected abstract Task SendCoreAsync(
Notification notification, CancellationToken ct);
}
public sealed class EmailNotificationSender(IEmailClient client)
: NotificationSender
{
protected override async Task SendCoreAsync(
Notification notification, CancellationToken ct)
{
await client.SendEmailAsync(
notification.Recipient, notification.Subject,
notification.Body, ct);
}
}
Liskov Substitution Principle (LSP)
Subtypes must be substitutable for their base types without altering program correctness. A subclass must honor the behavioral contract of its parent -- preconditions cannot be strengthened, postconditions cannot be weakened.
Anti-Pattern: Throwing in Override
public class FileStorage : IStorage
{
public virtual Stream OpenRead(string path) =>
File.OpenRead(path);
}
// WRONG -- ReadOnlyFileStorage violates the base contract by
// throwing on a method the base type supports
public class ReadOnlyFileStorage : FileStorage
{
public override Stream OpenRead(string path)
{
if (!File.Exists(path))
throw new InvalidOperationException(
"Cannot open files in read-only mode");
return base.OpenRead(path);
}
// Surprise: callers expecting FileStorage behavior get exceptions
}
Anti-Pattern: Collection Covariance Pitfall
// WRONG -- List<T> is not covariant; this compiles but causes runtime issues
IList<Animal> animals = new List<Dog>(); // Compile error (correctly)
// However, arrays ARE covariant in C# -- this compiles but throws at runtime:
Animal[] animals = new Dog[10];
animals[0] = new Cat(); // ArrayTypeMismatchException at runtime!
Fix: Use Covariant Interfaces
// IEnumerable<out T> and IReadOnlyList<out T> are covariant
IEnumerable<Animal> animals = new List<Dog>(); // Safe -- read-only
IReadOnlyList<Animal> readOnlyAnimals = new List<Dog>(); // Safe
// When you need mutability, keep the concrete type
List<Dog> dogs = [new Dog("Rex"), new Dog("Buddy")];
ProcessAnimals(dogs); // Pass to covariant parameter
void ProcessAnimals(IReadOnlyList<Animal> animals)
{
foreach (var animal in animals)
animal.Speak();
}
LSP Compliance Checklist
- Derived classes do not throw new exception types that the base does not declare
- Overrides do not add preconditions (e.g., null checks the base does not require)
- Overrides do not weaken postconditions (e.g., returning null when base guarantees non-null)
- Behavioral contracts are preserved: if
ICollection.Addsucceeds on the base, it must succeed on the derived type
Interface Segregation Principle (ISP)
Clients should not be forced to depend on methods they do not use. Prefer narrow, role-specific interfaces over wide "header" interfaces.
Anti-Pattern: Header Interface
// WRONG -- IWorker forces all implementations to support every capability
public interface IWorker
{
Task DoWorkAsync(CancellationToken ct);
void ClockIn();
void ClockOut();
Task<decimal> CalculatePayAsync();
void RequestTimeOff(DateRange range);
Task SubmitExpenseAsync(Expense expense);
}
// ContractWorker does not clock in/out or request time off
public class ContractWorker : IWorker
{
public Task DoWorkAsync(CancellationToken ct) => /* ... */;
public void ClockIn() => throw new NotSupportedException(); // ISP violation
public void ClockOut() => throw new NotSupportedException(); // ISP violation
public Task<decimal> CalculatePayAsync() => /* ... */;
public void RequestTimeOff(DateRange range) =>
throw new NotSupportedException(); // ISP violation
public Task SubmitExpenseAsync(Expense expense) =>
throw new NotSupportedException(); // ISP violation
}
Fix: Role Interfaces
public interface IWorkPerformer
{
Task DoWorkAsync(CancellationToken ct);
}
public interface ITimeTrackable
{
void ClockIn();
void ClockOut();
}
public interface IPayable
{
Task<decimal> CalculatePayAsync();
}
public interface ITimeOffEligible
{
void RequestTimeOff(DateRange range);
}
// FullTimeEmployee implements all applicable interfaces
public sealed class FullTimeEmployee :
IWorkPerformer, ITimeTrackable, IPayable, ITimeOffEligible
{
public Task DoWorkAsync(CancellationToken ct) => /* ... */;
public void ClockIn() { /* ... */ }
public void ClockOut() { /* ... */ }
public Task<decimal> CalculatePayAsync() => /* ... */;
public void RequestTimeOff(DateRange range) { /* ... */ }
}
// ContractWorker only implements what it needs
public sealed class ContractWorker : IWorkPerformer, IPayable
{
public Task DoWorkAsync(CancellationToken ct) => /* ... */;
public Task<decimal> CalculatePayAsync() => /* ... */;
}
Practical .NET ISP
The .NET BCL demonstrates ISP well:
| Wide Interface | Segregated Alternatives |
|---|---|
IList<T> (read + write) |
IReadOnlyList<T> (read only) |
ICollection<T> |
IReadOnlyCollection<T> |
IDictionary<K,V> |
IReadOnlyDictionary<K,V> |
Accept the narrowest interface your method actually needs:
// WRONG -- requires IList<T> but only reads
public decimal CalculateTotal(IList<OrderLine> lines) =>
lines.Sum(l => l.Price * l.Quantity);
// RIGHT -- accepts IReadOnlyList<T> since it only reads
public decimal CalculateTotal(IReadOnlyList<OrderLine> lines) =>
lines.Sum(l => l.Price * l.Quantity);
// BEST for iteration only -- accepts IEnumerable<T>
public decimal CalculateTotal(IEnumerable<OrderLine> lines) =>
lines.Sum(l => l.Price * l.Quantity);
Dependency Inversion Principle (DIP)
High-level modules should not depend on low-level modules. Both should depend on abstractions. Abstractions should not depend on details.
Anti-Pattern: Direct Dependency
// WRONG -- high-level OrderProcessor depends directly on low-level SqlOrderRepository
public sealed class OrderProcessor
{
private readonly SqlOrderRepository _repository = new();
private readonly SmtpEmailSender _emailSender = new();
public async Task ProcessAsync(Order order)
{
await _repository.SaveAsync(order); // Tight coupling to SQL
await _emailSender.SendAsync(order.Email, // Tight coupling to SMTP
"Order processed", $"Order {order.Id}");
}
}
Fix: Depend on Abstractions
public interface IOrderRepository
{
Task SaveAsync(Order order, CancellationToken ct = default);
Task<Order?> GetByIdAsync(string id, CancellationToken ct = default);
}
public interface INotificationService
{
Task NotifyAsync(string recipient, string subject,
string body, CancellationToken ct = default);
}
// High-level module depends on abstractions
public sealed class OrderProcessor(
IOrderRepository repository,
INotificationService notifier)
{
public async Task ProcessAsync(Order order, CancellationToken ct)
{
await repository.SaveAsync(order, ct);
await notifier.NotifyAsync(order.Email,
"Order processed", $"Order {order.Id}", ct);
}
}
// Low-level modules implement abstractions
public sealed class SqlOrderRepository(AppDbContext db) : IOrderRepository
{
public async Task SaveAsync(Order order, CancellationToken ct) =>
/* EF Core persistence */;
public async Task<Order?> GetByIdAsync(string id, CancellationToken ct) =>
await db.Orders.FindAsync([id], ct);
}
DI Registration
Register abstractions with Microsoft.Extensions.DependencyInjection. See [skill:dotnet-csharp-dependency-injection] for lifetime management, keyed services, and decoration patterns.
builder.Services.AddScoped<IOrderRepository, SqlOrderRepository>();
builder.Services.AddScoped<INotificationService, SmtpNotificationService>();
builder.Services.AddScoped<OrderProcessor>();
DIP Boundaries
Apply DIP at module boundaries, not everywhere:
- DO abstract infrastructure (database, email, file system, HTTP clients)
- DO abstract cross-cutting concerns (logging is already abstracted via
ILogger<T>) - DO NOT abstract simple value objects, DTOs, or internal implementation details
- DO NOT create
IFoo/Foopairs for every class -- only abstract where substitution adds value (testing, multiple implementations, or anticipated change)
DRY (Don't Repeat Yourself)
Every piece of knowledge should have a single, authoritative representation. But DRY is about knowledge duplication, not code duplication.
When to Apply DRY
Apply DRY when two pieces of code represent the same concept and must change together:
// WRONG -- tax rate duplicated across two services
public sealed class InvoiceService
{
public decimal CalculateTax(decimal amount) => amount * 0.08m;
}
public sealed class QuoteService
{
public decimal EstimateTax(decimal amount) => amount * 0.08m;
}
// RIGHT -- single source of truth
public static class TaxRates
{
public const decimal StandardRate = 0.08m;
}
Rule of Three
Do not abstract prematurely. Wait until you see the same pattern three times before extracting a shared abstraction:
- First occurrence -- write it inline
- Second occurrence -- note the duplication but keep it (the two usages may diverge)
- Third occurrence -- extract a shared method, class, or utility
When Duplication Is Acceptable
Not all code similarity represents knowledge duplication:
// These look similar but represent DIFFERENT business concepts
// They will evolve independently -- DO NOT merge them
public sealed class CustomerValidator
{
public bool IsValid(Customer customer) =>
!string.IsNullOrEmpty(customer.Name) &&
!string.IsNullOrEmpty(customer.Email);
}
public sealed class SupplierValidator
{
public bool IsValid(Supplier supplier) =>
!string.IsNullOrEmpty(supplier.Name) &&
!string.IsNullOrEmpty(supplier.ContactEmail);
}
Acceptable duplication scenarios:
- Test setup code that looks similar across test classes (coupling tests to shared helpers makes them fragile)
- DTOs for different API versions (V1 and V2 may share fields now but diverge later)
- Configuration for different environments (dev and prod configs that happen to be similar today)
- Mapping code between layers (coupling layers to share mappers defeats the purpose of separate layers)
Abstracting Shared Behavior
When you do extract, prefer composition over inheritance:
// Prefer: composition via a shared utility
public static class StringValidation
{
public static bool IsNonEmpty(string? value) =>
!string.IsNullOrWhiteSpace(value);
}
// Over: inheritance via a base class
// (couples validators to a shared base, harder to test independently)
Applying the Principles Together
Decision Guide
| Symptom | Likely Violation | Fix |
|---|---|---|
| Class described with "and" | SRP | Split into focused classes |
| Modifying existing code to add features | OCP | Use strategy/plugin pattern |
NotSupportedException in overrides |
LSP | Redesign hierarchy or use composition |
| Empty/throwing interface methods | ISP | Split into role interfaces |
new keyword for dependencies |
DIP | Inject via constructor |
| Magic numbers/strings in multiple files | DRY | Extract constants or config |
| Copy-pasted code blocks (3+) | DRY | Extract shared method |
SRP Compliance Test
For each class, answer these questions:
- One-sentence test: Can you describe the class's purpose in one sentence without "and" or "or"?
- Change-reason test: List all reasons this class might need to change. If more than one, consider splitting.
- Dependency count test: Does the constructor take more than 3-4 dependencies? High parameter counts often signal multiple responsibilities.
Agent Gotchas
- Do not create
IFoo/Foopairs for every class. DIP calls for abstractions at module boundaries (infrastructure, external services), not for every internal class. Unnecessary interfaces add indirection without value and clutter the codebase. - Do not merge similar-looking code from different bounded contexts. Two validators or DTOs that look alike but serve different business concepts should remain separate. Premature DRY creates coupling between concepts that evolve independently.
- Do not use inheritance to share behavior between unrelated types. Prefer composition (injecting a shared service or using extension methods) over inheriting from a common base class. Inheritance creates tight coupling and makes LSP violations more likely.
- Fat controllers and god classes are SRP violations. When generating endpoint handlers, keep them thin -- delegate to dedicated services for validation, business logic, and persistence. Apply the "one sentence" test to each class.
- Switch statements on type discriminators violate OCP. Replace them with polymorphism (strategy pattern, interface dispatch) so new types can be added without modifying existing code.
- Array covariance in C# is unsafe.
Animal[] animals = new Dog[10]compiles but throwsArrayTypeMismatchExceptionat runtime when adding non-Dog elements. UseIReadOnlyList<T>orIEnumerable<T>for covariant read-only access. - Accept the narrowest interface type your method needs. Use
IEnumerable<T>for iteration,IReadOnlyList<T>for indexed read access, andIList<T>only when mutation is required. This follows ISP and makes methods more reusable.
Knowledge Sources
SOLID and DRY guidance in this skill is grounded in publicly available content from:
- Steve Smith (Ardalis) SOLID Principles -- Practical SOLID application in .NET with guard clause patterns, specification pattern for OCP compliance, and clean architecture layering that enforces DIP at project boundaries. Source: https://ardalis.com/
- Jimmy Bogard's Domain-Driven Design Patterns -- Rich domain model guidance that applies SRP to aggregate design (one aggregate root per bounded context) and OCP to domain event handling (new handlers without modifying existing ones). Note: MediatR is commercial for commercial use; apply the patterns with built-in mechanisms where possible. Source: https://www.jimmybogard.com/
Note: This skill applies publicly documented guidance. It does not represent or speak for the named sources.
References
- SOLID Principles in C#
- Dependency Injection in .NET
- Covariance and Contravariance in Generics
- Clean Architecture (Ardalis)
Attribution
Adapted from Aaronontheweb/dotnet-skills (MIT license).