SOLID Violations in C#: How to Recognize and Fix Each One
Solid violations c# are sneaky. Learning to recognize solid violations c# early prevents the slow rot from compounding. They don't show up as compile errors or immediate crashes. They show up as friction -- the class that takes 20 minutes to understand, the PR that touches eight files to add one feature, the bug that reappears three weeks after being "fixed." SOLID principles exist to prevent exactly that slow rot. But knowing the principles in theory and recognizing violations in real code are two very different skills.
This article is focused on recognition and repair. Each section covers one principle, shows a realistic violation you'll encounter in production, and then shows the refactored version. By the end, you'll have a mental model for spotting these violations during code review -- before they compound into something much worse.
SRP Violation -- The God Class
The God Class is one of the most common solid violations c# teams encounter. The Single Responsibility Principle says a class should have one reason to change.The violation is the God Class -- a class that has a dozen reasons to change.
The OrderProcessor below does validation, persistence, email notifications, logging, and business rules. All in one place. It's the kind of class that grows organically as features get added, and nobody notices until it's 800 lines long.
// ❌ SRP Violation -- OrderProcessor does too much
public sealed class OrderProcessor
{
private readonly string _connectionString;
public OrderProcessor(string connectionString)
{
_connectionString = connectionString;
}
public void ProcessOrder(Order order)
{
// Validation
if (order.Items.Count == 0)
throw new InvalidOperationException("Order must have items.");
if (order.CustomerId == Guid.Empty)
throw new InvalidOperationException("Customer ID is required.");
// Business rules
decimal total = order.Items.Sum(i => i.Price * i.Quantity);
if (total > 10_000)
order.RequiresApproval = true;
// Persistence -- direct SQL
using var connection = new SqlConnection(_connectionString);
connection.Open();
using var cmd = connection.CreateCommand();
cmd.CommandText = "INSERT INTO Orders (Id, CustomerId, Total) VALUES (@id, @cid, @total)";
cmd.Parameters.AddWithValue("@id", order.Id);
cmd.Parameters.AddWithValue("@cid", order.CustomerId);
cmd.Parameters.AddWithValue("@total", total);
cmd.ExecuteNonQuery();
// Email notification
var smtp = new SmtpClient("smtp.company.com");
smtp.Send("[email protected]", "[email protected]",
$"New Order {order.Id}", $"Total: {total:C}");
// Logging
File.AppendAllText("orders.log",
$"[{DateTime.UtcNow:O}] Order {order.Id} processed.
");
}
}
Every time the email template changes, the schema changes, the validation rules change, or the logging format changes -- this class changes. That's five separate reasons. Five different teams that might edit the same file on the same day.
A common fix is to extract each concern into a focused class.
// ✅ SRP Compliant -- each class has one job
public interface IOrderValidator
{
void Validate(Order order);
}
public interface IOrderRepository
{
Task SaveAsync(Order order, decimal total, CancellationToken ct = default);
}
public interface IOrderNotifier
{
Task NotifyAsync(Order order, decimal total, CancellationToken ct = default);
}
public sealed class OrderBusinessRules
{
public decimal CalculateTotal(Order order) =>
order.Items.Sum(i => i.Price * i.Quantity);
public void ApplyApprovalPolicy(Order order, decimal total)
{
if (total > 10_000)
order.RequiresApproval = true;
}
}
public sealed class OrderProcessor(
IOrderValidator validator,
IOrderRepository repository,
IOrderNotifier notifier,
OrderBusinessRules rules,
ILogger<OrderProcessor> logger)
{
public async Task ProcessOrderAsync(Order order, CancellationToken ct = default)
{
validator.Validate(order);
decimal total = rules.CalculateTotal(order);
rules.ApplyApprovalPolicy(order, total);
await repository.SaveAsync(order, total, ct);
await notifier.NotifyAsync(order, total, ct);
logger.LogInformation("Order {OrderId} processed successfully.", order.Id);
}
}
Now each class changes for exactly one reason. The OrderProcessor itself becomes a thin coordinator. For structured logging patterns that complement this approach, the Logging in .NET Complete Guide covers the ILogger<T> patterns used above.
OCP Violation -- The Brittle Switch
This is a solid violations c# pattern -- the brittle switch. The Open/Closed Principle says code should be open for extension but closed for modification.The classic violation is a switch statement (or a long if/else if chain) that must be modified every time a new case is added.
// ❌ OCP Violation -- every new payment type requires editing this class
public sealed class PaymentService
{
public decimal ProcessPayment(string paymentType, decimal amount)
{
return paymentType switch
{
"CreditCard" => amount * 1.02m, // 2% fee
"PayPal" => amount * 1.035m, // 3.5% fee
"BankWire" => amount + 15m, // flat fee
_ => throw new NotSupportedException($"Unknown payment type: {paymentType}")
};
}
}
Every new payment method requires opening PaymentService and editing it. This violates OCP, but it also introduces risk -- modifying code that already works is exactly how you break things that were working.
A common fix uses polymorphism. Define an abstraction, implement it for each payment type, and use a registry pattern to select the right implementation at runtime.
// ✅ OCP Compliant -- add new payment type without modifying existing code
public interface IPaymentProcessor
{
string PaymentType { get; }
decimal CalculateFee(decimal amount);
}
public sealed class CreditCardProcessor : IPaymentProcessor
{
public string PaymentType => "CreditCard";
public decimal CalculateFee(decimal amount) => amount * 1.02m;
}
public sealed class PayPalProcessor : IPaymentProcessor
{
public string PaymentType => "PayPal";
public decimal CalculateFee(decimal amount) => amount * 1.035m;
}
public sealed class BankWireProcessor : IPaymentProcessor
{
public string PaymentType => "BankWire";
public decimal CalculateFee(decimal amount) => amount + 15m;
}
public sealed class PaymentService(IEnumerable<IPaymentProcessor> processors)
{
private readonly Dictionary<string, IPaymentProcessor> _processors =
processors.ToDictionary(p => p.PaymentType);
public decimal ProcessPayment(string paymentType, decimal amount)
{
if (!_processors.TryGetValue(paymentType, out var processor))
throw new NotSupportedException($"Unknown payment type: {paymentType}");
return processor.CalculateFee(amount);
}
}
Adding "Crypto" now means writing a new class. Nothing else changes. Note that registry + polymorphism is one common approach -- data tables, pattern matching, or configuration-driven rules can also work depending on the context and the complexity of the dispatch logic. The Visitor vs Strategy Pattern article explores the tradeoffs between strategy dispatch like this and visitor-based approaches when your dispatch logic gets more complex.
LSP Violation -- The Leaky Override
Leaky overrides represent one of the trickiest solid violations c# to spot. The Liskov Substitution Principle says you should be able to substitute a subtype anywhere a base type is used without breaking the program.The violation shows up as an override that throws, silently ignores behavior, or returns a sentinel value to indicate "not supported."
The Bird/Penguin example is famous for a reason. It's obvious in isolation but painful in a real codebase.
// ❌ LSP Violation -- Penguin IS-A Bird, but cannot fly
public class Bird
{
public virtual void Fly() => Console.WriteLine("Flap flap!");
}
public class Penguin : Bird
{
public override void Fly() =>
throw new NotSupportedException("Penguins cannot fly.");
}
// Breaks at runtime -- not at compile time
public static void MakeBirdFly(Bird bird) => bird.Fly();
var penguin = new Penguin();
MakeBirdFly(penguin); // 💥 NotSupportedException
A common fix is to rethink the hierarchy. Don't model IS-A based on real-world categories -- model it based on behavioral contracts. If Penguin can't satisfy the full contract of Bird, it shouldn't inherit from Bird the way it does.
// ✅ LSP Compliant -- abstraction is split by capability
public abstract class Bird
{
public abstract string Name { get; }
public abstract void Move();
}
public interface IFlyingBird
{
void Fly();
}
public sealed class Sparrow : Bird, IFlyingBird
{
public override string Name => "Sparrow";
public override void Move() => Fly();
public void Fly() => Console.WriteLine("Sparrow soars.");
}
public sealed class Penguin : Bird
{
public override string Name => "Penguin";
public override void Move() => Console.WriteLine("Penguin waddles.");
}
// Code that needs flying behavior asks for IFlyingBird explicitly
public static void MakeBirdFly(IFlyingBird bird) => bird.Fly();
Now the compiler enforces the contract. You can't accidentally pass a Penguin where flying is required. LSP violations are especially insidious because they produce runtime failures -- not compile-time errors -- often deep in code paths you don't regularly exercise.
ISP Violation -- The Bloated Interface
This is a solid violations c# pattern called the bloated interface. The Interface Segregation Principle says clients should not be forced to depend on methods they don't use.The violation is a fat interface where most implementations stub half the methods with NotImplementedException.
// ❌ ISP Violation -- not every repository needs all 8 methods
public interface IRepository<T>
{
T? GetById(Guid id);
IEnumerable<T> GetAll();
IEnumerable<T> Search(string query);
void Add(T entity);
void Update(T entity);
void Delete(Guid id);
void BulkInsert(IEnumerable<T> entities);
void Archive(Guid id);
}
// Read-only cache implementation -- half of this makes no sense
public sealed class CachedProductRepository : IRepository<Product>
{
public Product? GetById(Guid id) => /* read from cache */ null;
public IEnumerable<Product> GetAll() => /* read from cache */ [];
public IEnumerable<Product> Search(string query) => /* search cache */ [];
public void Add(Product entity) =>
throw new NotImplementedException("Cache is read-only.");
public void Update(Product entity) =>
throw new NotImplementedException("Cache is read-only.");
public void Delete(Guid id) =>
throw new NotImplementedException("Cache is read-only.");
public void BulkInsert(IEnumerable<Product> entities) =>
throw new NotImplementedException("Cache is read-only.");
public void Archive(Guid id) =>
throw new NotImplementedException("Cache is read-only.");
}
A common fix is to split the interface into cohesive, focused pieces. Read-only consumers depend on IReadRepository<T>. Write consumers depend on IWriteRepository<T>. Implementations only implement what they actually support.
// ✅ ISP Compliant -- interfaces segregated by capability
public interface IReadRepository<T>
{
T? GetById(Guid id);
IEnumerable<T> GetAll();
IEnumerable<T> Search(string query);
}
public interface IWriteRepository<T>
{
void Add(T entity);
void Update(T entity);
void Delete(Guid id);
}
public interface IBulkRepository<T>
{
void BulkInsert(IEnumerable<T> entities);
void Archive(Guid id);
}
// Read-only cache only implements what it supports
public sealed class CachedProductRepository : IReadRepository<Product>
{
public Product? GetById(Guid id) => /* read from cache */ null;
public IEnumerable<Product> GetAll() => /* read from cache */ [];
public IEnumerable<Product> Search(string query) => /* search cache */ [];
}
// Full SQL repository implements everything
public sealed class SqlProductRepository :
IReadRepository<Product>,
IWriteRepository<Product>,
IBulkRepository<Product>
{
public Product? GetById(Guid id) => /* SQL read */ null;
public IEnumerable<Product> GetAll() => /* SQL read */ [];
public IEnumerable<Product> Search(string query) => /* SQL search */ [];
public void Add(Product entity) { /* SQL insert */ }
public void Update(Product entity) { /* SQL update */ }
public void Delete(Guid id) { /* SQL delete */ }
public void BulkInsert(IEnumerable<Product> entities) { /* batch insert */ }
public void Archive(Guid id) { /* soft delete */ }
}
Smaller interfaces also make testing dramatically easier. When a service depends on IReadRepository<T>, you mock three methods instead of eight. A NotImplementedException in a concrete class is often a strong signal worth investigating -- it frequently points to an interface that has grown beyond a single cohesive role.
DIP Violation -- The Hardcoded Dependency
Hardcoded dependencies are among the most damaging solid violations c# in production code. The Dependency Inversion Principle says high-level modules should not depend on low-level modules -- both should depend on abstractions.The violation is any new keyword creating an infrastructure dependency inside a class that contains business logic.
// ❌ DIP Violation -- business logic directly creates infrastructure
public sealed class ReportingService
{
public IEnumerable<string> GetActiveUserReports()
{
// Hardcoded SQL inside business logic
using var connection = new SqlConnection("Server=prod-sql;Database=AppDb;...");
connection.Open();
using var cmd = connection.CreateCommand();
cmd.CommandText = "SELECT Name FROM Users WHERE IsActive = 1";
using var reader = cmd.ExecuteReader();
var results = new List<string>();
while (reader.Read())
results.Add(reader.GetString(0));
return results;
}
}
You cannot unit test this. You cannot swap databases. You cannot run it against an in-memory test store without modifying production code. The business logic and the infrastructure are fused together.
Some concrete dependencies are acceptable if they represent stable, pure collaborators rather than infrastructure boundaries. But SqlConnection and similar infrastructure types are rarely in that category -- they cross boundaries that change independently of business logic.
A common fix uses constructor injection through abstractions. This is exactly what How DI Containers Use Reflection Internally explains -- .NET's DI container resolves these abstractions for you at runtime, keeping your business logic clean.
// ✅ DIP Compliant -- depend on abstractions, inject via constructor
public interface IUserRepository
{
IEnumerable<string> GetActiveUserNames();
}
public sealed class SqlUserRepository(IDbConnection connection) : IUserRepository
{
public IEnumerable<string> GetActiveUserNames()
{
if (connection.State != ConnectionState.Open)
connection.Open();
using var cmd = connection.CreateCommand();
cmd.CommandText = "SELECT Name FROM Users WHERE IsActive = 1";
using var reader = cmd.ExecuteReader();
var results = new List<string>();
while (reader.Read())
results.Add(reader.GetString(0));
return results;
}
}
public sealed class ReportingService(IUserRepository userRepository)
{
public IEnumerable<string> GetActiveUserReports() =>
userRepository.GetActiveUserNames();
}
The ReportingService now has zero knowledge of SQL, connection strings, or any infrastructure concern. In tests, inject a mock IUserRepository. In production, the DI container wires up SqlUserRepository automatically. Clean separation -- no surgery required.
How Violations Compound
Understanding how solid violations c# compound is essential for preventing them. This is the part most tutorials skip. SOLID violations in C# rarely appear in isolation. They compound.
SRP breaks first. A class grows too large and takes on too many responsibilities. Now it's hard to change without introducing bugs, so developers start working around it instead of refactoring it. They add if/else branches for new cases. That's OCP breaking -- the class is no longer closed for modification.
As the class grows, inheritance hierarchies get stretched to accommodate variants. Someone adds a LiteOrderProcessor that inherits from OrderProcessor but throws on the payment methods. LSP just broke. Interfaces get fat to accommodate all the variants. ISP breaks. And eventually the whole mess starts directly instantiating its dependencies because refactoring the DI setup felt like too much work. DIP breaks.
A God Class often exhibits multiple SOLID tensions simultaneously -- this illustrates how violations compound, not a deterministic law. That's why refactoring toward SOLID is most valuable early -- when the first violation appears, not after all five are embedded and entangled.
Design patterns can help interrupt this cascade. The Mediator Design Pattern in C# is effective for SRP and DIP -- it decouples components that would otherwise accumulate shared state. The Facade Design Pattern helps manage complexity when you have a legitimate need to coordinate multiple subsystems without violating SRP. The Visitor Design Pattern in C# is one common way to support OCP when you're dispatching behavior based on type. The Chain of Responsibility Design Pattern handles the pipeline-of-concerns smell that often appears after SRP is violated. And the Proxy Design Pattern is excellent for DIP fixes when you need to wrap a concrete dependency behind an abstraction without changing the calling code.
Code Review Checklist for solid violations c#
Use these questions as diagnostic prompts when reviewing code -- or when reviewing your own code before submitting a PR. They highlight signals worth investigating, not automatic proof of a violation.
Single Responsibility
Start here -- SRP violations are the most common and the most contagious.
- Does this class have more than one reason to change?
- Would two different teams ever need to edit this class independently?
- Does the class name end in "Manager," "Helper," or "Processor" and do five different things?
Open/Closed
If adding a new feature requires modifying a switch statement or an if/else chain, OCP is probably violated.
- If I need to add a new type or case, do I have to modify existing code?
- Is there a
switchorif/else ifchain that maps type names to behavior?
Liskov Substitution
A NotImplementedException in an override is a strong signal worth investigating. In most cases it points to a hierarchy that needs redesign -- though context matters, such as scaffolding or evolving APIs.
- Does any override throw
NotImplementedExceptionorNotSupportedException? - Does a subtype silently ignore part of the base class contract?
- Could code written against the base type ever behave unexpectedly when given this subtype?
Interface Segregation
If a class implements five methods from an interface but only two are meaningful to it, the interface is too fat.
- Does any class implement an interface but leave methods as
throw new NotImplementedException()? - Are there consumers that use only two or three methods of a six-method interface?
- Is the interface named after a role, or is it named after the class that implements it?
Dependency Inversion
new SomeInfrastructureType() inside business logic is the clearest DIP violation. Inject it instead.
- Is
new SomeConcreteInfrastructureClass()called inside a class that contains business logic? - Does any class take a dependency on a specific database library, HTTP client, or file path?
- Are dependencies injected via constructor, or are they created internally?
FAQ
What is the most common SOLID violation in C# codebases?
SRP violations -- specifically the God Class -- are the most common solid violations c# teams encounter in production. They emerge naturally as features get added over time. Each addition seems small. The problem accumulates invisibly until refactoring becomes a multi-day project. The trigger is usually a class name that ends in "Manager" or "Service" and a file that's over 400 lines long.
Can you have too many small classes when fixing SOLID violations?
Yes. Over-decomposition is a real problem. If you split a class into 15 tiny classes that each contain one line of logic, you've traded one problem for another -- now the mental model for understanding the system requires navigating 15 files. The rule is one reason to change, not one method per class. Use judgment. Complexity you create is still complexity.
Are SOLID violations always worth fixing?
Not always. In a prototype, a throwaway script, or a deadline-critical hotfix, structural purity is a lower priority than shipping. The question is whether the code will be maintained. If it will -- fix the violations before they compound. The cost of refactoring a violation doubles every time another layer is built on top of it.
How do I spot LSP violations in code review?
Look for overrides that throw exceptions, overrides with empty bodies, and overrides that return sentinel values like null or -1 to indicate "not supported." Any override that doesn't fully honor the behavioral contract of its base type is an LSP violation. The subtler ones are overrides that partially honor the contract but silently skip some of the expected side effects.
Does dependency injection automatically mean DIP compliance?
No. You can inject dependencies that are still concrete classes. DIP is about depending on abstractions -- interfaces or abstract classes -- not just about using constructor injection. Injecting SqlConnection directly is still a DIP violation even if the DI container created it. The key question is: could you swap out this dependency for a different implementation without changing the class under test?
How do SOLID violations affect testability?
Severely. God Classes are hard to test because each test must cover too many concerns at once. DIP violations make unit testing impossible without real infrastructure. ISP violations make mocking painful because you must stub methods you don't care about. Every SOLID violation is also a testability violation -- that's often the most practical signal that a principle has been broken.
Is the Dependency Inversion Principle the same as Dependency Injection?
No -- they're related but distinct. Dependency Inversion is a design principle: high-level modules should depend on abstractions, not concretions. Dependency Injection is a technique for providing those abstractions at runtime. DIP tells you what to design. DI tells you how to wire it up. You can violate DIP while using DI (by injecting concrete classes). You can also satisfy DIP without a DI container (by manually passing abstractions through constructors).
Conclusion
Solid violations c# tend to hide in plain sight. They're not syntax errors or runtime panics. They're structural choices that felt reasonable at the time and gradually made the codebase harder to extend, test, and understand. Recognizing them early -- in code review, in architecture discussions, in your own PRs before you submit them -- is one of the most valuable skills you can develop as a .NET developer.
The signals are consistent across every codebase: classes with too many reasons to change, switch statements that keep growing, overrides that throw, fat interfaces littered with NotImplementedException, and business logic that instantiates its own infrastructure. When you see those signals, you now know exactly what you're looking at and how to fix it.
Keep that code review checklist close. Use it consistently. The violations you catch early cost a fraction of what violations cost when they've been compounding for six months.

