Single Responsibility Principle
The SRP states that a class should have only one reason to change, meaning it should only have one responsibility or job.
Violating SRP
public class Journal
{
private readonly List<string> entries = new();
public void AddEntry(string text)
{
entries.Add(text);
}
public void RemoveEntry(int index)
{
entries.RemoveAt(index);
}
public void Save(string filename, bool overwrite = false)
{
File.WriteAllText(filename, ToString());
}
}
It makes sense to have this method as part of the Journal class because adding a journal entry is something the journal actually needs to do. It is the journal’s responsibility to keep entries, so anything related to that is fair game.
This approach is problematic. The journal’s responsibility is to keep journal entries, not to write them to disk. If you add the persistence functionality to Journal and similar classes, any change in the approach to persistence (say, you decide to write to the cloud instead of disk) would require lots of tiny changes in each of the affected classes.
SRP
public class Journal
{
private readonly List<string> entries = new();
public void AddEntry(string text)
{
entries.Add(text);
}
public void RemoveEntry(int index)
{
entries.RemoveAt(index);
}
public override string ToString()
{
return string.Join(Environment.NewLine, entries);
}
}
public class JournalPersistence
{
public void SaveToFile(Journal journal, string filename, bool overwrite = false)
{
if (overwrite || !File.Exists(filename))
{
File.WriteAllText(filename, journal.ToString());
}
}
public Journal LoadFromFile(string filename)
{
var entries = File.ReadAllLines(filename).ToList();
var journal = new Journal();
foreach (var entry in entries)
{
journal.AddEntry(entry);
}
return journal;
}
}
And this is precisely what we mean by Single Responsibility: each class has only one responsibility and therefore has only one reason to change. Journal would need to change only if there’s something more that needs to be done with respect to in-memory storage of entries – for example, you might want each entry prefixed by a timestamp, so you would change the Add() method to do exactly that. On the other hand, if you wanted to change the persistence mechanic, this would be changed in PersistenceManager.
An extreme example of an anti-pattern that violates the SRP is called a God Object. A God Object is a huge class that tries to handle as many concerns as possible.
Open-Closed Principle
A class should be open for extension, but closed for modification, meaning that it should allow adding new features without changing the existing code.
Violating OCP
Suppose we have an (entirely hypothetical) range of products in a database. Each product has a color and size and is defined as:
public enum Color { Red, Green, Blue }
public enum Size { Small, Medium, Large, Huge }
public record Product(string Name, Color Color, Size Size);
Now, we want to provide certain filtering capabilities for a given set of products. We make a ProductFilter service class. To support filtering products by color, we implement it as follows:
public class ProductFilter
{
public IEnumerable<Product> FilterByColor
(IEnumerable<Product> products, Color color)
{
foreach (var p in products)
if (p.Color == color)
yield return p;
}
}
Our current approach of filtering items by color is all well and good, though of course it could be greatly simplified with the use of LINQ. So our code goes into production, but unfortunately, sometime later, the boss comes in and asks us to implement filtering by size too. So we jump back into ProductFilter.cs, add the following code, and recompile:
public class ProductFilter
{
public IEnumerable<Product> FilterByColor
(IEnumerable<Product> products, Color color)
{
foreach (var p in products)
if (p.Color == color)
yield return p;
}
public IEnumerable<Product> FilterBySize
(IEnumerable<Product> products, Size size)
{
foreach (var p in products)
if (p.Size == size)
yield return p;
}
}
This feels like outright duplication, doesn’t it? Why don’t we just write a general method that takes a predicate (i.e., a Predicate
Furthermore, you might want to restrict the criteria one can filter on. For example, if you look at Amazon or a similar online store, you are only allowed to perform filtering on a finite set of criteria. Those criteria can be added or removed by Amazon if they find that, say, sorting by number of reviews interferes with the bottom line.
Okay, so our code goes into production, but once again, the boss comes back and tells us that now there’s a need to search by both size and color. So what are we to do but add another function?
public class ProductFilter
{
public IEnumerable<Product> FilterByColor
(IEnumerable<Product> products, Color color)
{
foreach (var p in products)
if (p.Color == color)
yield return p;
}
public IEnumerable<Product> FilterBySize
(IEnumerable<Product> products, Size size)
{
foreach (var p in products)
if (p.Size == size)
yield return p;
}
public IEnumerable<Product> FilterBySizeAndColor(
IEnumerable<Product> products,
Size size, Color color)
{
foreach (var p in products)
if (p.Size == size && p.Color == color)
yield return p;
}
}
OCP
What we want, from the preceding scenario, is to enforce the Open-Closed Principle that states that a type is open for extension but closed for modification. In other words, we want filtering that is extensible (perhaps in a different assembly or Extension methods) without having to modify it (and recompiling something that already works and may have been shipped to clients).
public class BaseClass
{
// Base class implementation
}
public class Drived : BaseClass
{
public void Print()
{
// Implementation of the Print method
Console.WriteLine(nameof(Drived));
}
}
public static class BaseClassExtensions
{
public static void Print(this BaseClass baseClass) // extended `BaseClass` type without midify it, using extension method.
{
// Implementation of the Print method
Console.WriteLine("extension");
}
}
How can we achieve it? Well, first of all, we conceptually separate (SRP!) our filtering process into two parts: a filter (a construct that takes all items and only returns some) and a specification (a predicate to apply to a data element). We can make a very simple definition of a specification interface:
public interface ISpecification<T>
{
bool IsSatisfied(T item);
}
In this interface, type T is whatever we choose it to be: it can certainly be a Product, but it can also be something else. This makes the entire approach reusable. Next up, we need a way of filtering based on an ISpecification<T> – this is done by defining, you guessed it, an IFilter<T>:
public interface IFilter<T>
{
IEnumerable<T> Filter(IEnumerable<T> items,
ISpecification<T> spec);
}
Again, all we are doing is specifying the signature for a method called Filter() that takes all the items and a specification and returns only those items that conform to the specification.
Based on this interface, the implementation of an improved filter is really simple:
public class BetterFilter : IFilter<Product>
{
public IEnumerable<Product> Filter(IEnumerable<Product> items,
ISpecification<Product> spec)
{
foreach (var i in items)
if (spec.IsSatisfied(i))
yield return i;
}
}
Again, you can think of an ISpecification<T> that’s being passed in as a strongly typed equivalent of a Predicate<T> that has a finite set of concrete implementations suitable for the problem domain.
Now, here’s the easy part. To make a color filter, you make a ColorSpecification:
public class ColorSpecification : ISpecification<Product>
{
private Color color;
public ColorSpecification(Color color)
{
this.color = color;
}
public bool IsSatisfied(Product p)
{
return p.Color == color;
}
}
public class SizeSpecification : ISpecification<Product>
{
private Size size;
public SizeSpecification(Size size)
{
this.Size = size;
}
public bool IsSatisfied(Product p)
{
return p.Size == size;
}
}
public class AndSpecification<T> : ISpecification<T>
{
private readonly ISpecification<T> first, second;
public AndSpecification(ISpecification<T> first, ISpecification<T> second)
{
this.first = first;
this.second = second;
}
public override bool IsSatisfied(T t)
{
return first.IsSatisfied(t) && second.IsSatisfied(t);
}
}
var apple = new Product("Apple", Color.Green, Size.Small);
var tree = new Product("Tree", Color.Green, Size.Large);
var house = new Product("House", Color.Blue, Size.Large);
Product[] products = {apple, tree, house};
var bf = new BetterFilter();
foreach (var p in bf.Filter(products,
new AndSpecification<Product>(
new ColorSpecification(Color.Green),
new SizeSpecification(Size.Large))))
{
WriteLine($"{p.Name} is large");
}
This was a lot of code to do something seemingly simple, but the benefits are well worth it. The only really annoying part is having to specify the generic argument to AndSpecification – remember, unlike the color/size specifications, the combinator isn’t constrained to the Product type.
Keep in mind that, thanks to the power of C#, you can simply introduce an operator & (important: single ampersand here – && is a byproduct) for two ISpecification
public abstract class ISpecification<T>
{
public abstract bool IsSatisfied(T p);
public static ISpecification<T> operator &(
ISpecification<T> first, ISpecification<T> second)
{
return new AndSpecification<T>(first, second);
}
}
If you now avoid making extra variables for size/color specifications, the composite specification can be reduced to a single line:
var largeGreenSpec = new ColorSpecification(Color.Green)
& new SizeSpecification(Size.Large);
Naturally, you can take this approach to extreme by defining extension methods on all pairs of possible specifications.
public static class CriteriaExtensions
{
public static AndSpecification<Product> And(this Color color, Size size)
{
return new AndSpecification<Product>(
new ColorSpecification(color),
new SizeSpecification(size));
}
}
use it:
var largeGreenSpec = Color.Green.And(Size.Large);
Liskov Substitution Principle
A subclass should be able to replace its superclass without breaking the functionality, meaning that it should follow the contract of the superclass and not introduce any unexpected behavior.
Violating LSP
public class Rectangle
{
public int Width { get; set; }
public int Height { get; set; }
public Rectangle() { }
public Rectangle(int width, int height)
{
Width = width;
Height = height;
}
public int Area => Width * Height;
}
public class Square : Rectangle
{
public Square(int side)
{
Width = Height = side;
}
public new int Width
{
set { base.Width = base.Height = value; }
}
public new int Height
{
set { base.Width = base.Height = value; }
}
}
public static void UseIt(Rectangle r)
{
r.Height = 10;
Console.WriteLine($"Expected area of {10 * r.Width}, got {r.Area}");
}
var rc = new Rectangle(2,3);
UseIt(rc);
// Expected area of 20, got 20
var sq = new Square(5);
UseIt(sq);
// Expected area of 100, got 50
So the problem here is that although UseIt() is happy to take any Rectangle class, it fails to take a Square because the behaviors inside Square break its operation.
LSP
public abstract class Shape
{
public virtual int Height { get; set; }
public virtual int Width { get; set; }
public int Area => Height * Width;
}
public class Square : Shape
{
public Square(int side)
{
Width = Height = side;
}
public override int Width
{
set { base.Width = base.Height = value; }
}
public override int Height
{
set { base.Width = base.Height = value; }
}
}
public class Rectangle: Shape
{
public Rectangle(int width, int height)
{
Width = width;
Height = height;
}
}
public static void UseIt(Shape r)
{
r.Height = 10;
Console.WriteLine($"Expected area of {10 * r.Width}, got {r.Area}");
}
var rc = new Rectangle(2,3);
UseIt(rc);
// Expected area of 20, got 20
var sq = new Square(5);
UseIt(sq);
// Expected area of 100, got 100
Interface Segregation Principle
A class should not depend on methods that it does not use, meaning that it should have multiple, specific interfaces rather than one general interface.
Violating ISP
public interface IMachine
{
void Print(Document d);
void Fax(Document d);
void Scan(Document d);
}
This is a problem. The reason it is a problem is that some implementor of this interface might not need scanning or faxing, just printing. And yet, you are forcing them to implement those extra features: sure, they can all be no-op, but why bother with this?
A typical example would be a good old-fashioned printer that doesn’t have any scanning or fax functionality. Implementing the IMachine interface in this situation becomes a real challenge. What’s particularly frustrating about this situation is there is no correct way of leaving things unimplemented – this is actually a good indicator that interfaces are poorly segregated. I mean, sure, you can throw an exception, and we even have a dedicated exception precisely for this purpose:
public class OldFashionedPrinter : IMachine
{
public void Fax(Document d)
{
throw new NotImplementedException();
}
public void Print(Document d)
{
//yep
}
public void Scan(Document d)
{
// left empty
}
}
But you are still confusing the user! They can see OldFashionedPrinter.Fax() as part of the API, so they can be forgiven for thinking that this type of printer can fax too! So what else can you do? Well, you can just leave the extra methods as no-op (empty), just like the Scan() method. This approach violates the Principle of Least Surprise (also known as the Principle of Least Astonishment, POLA): your users want things to be as predictable as you can possibly make them. And neither a method that throws by default nor a method that does nothing is the most predictable solution – even if you make it explicit in the documentation!
The only option that would categorically work at compile time is the nuclear option of marking all unnecessary methods obsolete:
[Obsolete("Not supported", true)]
public void Scan(Document d)
{
throw new System.NotImplementedException();
}
This will prevent compilation if someone does try to use OldFashionedPrinter.Scan(). In fact, good IDEs will recognize this ahead of time and will often cross out the method as you call it to indicate that it’s not going to work. The only issue with this approach is that it’s deeply unidiomatic: the method isn’t really obsolete – it’s unimplemented. Stop lying to the client!
ISP
So what the Interface Segregation Principle suggests you do instead is split up interfaces, so that implementors can pick and choose depending on their needs.
public interface IPrinter
{
void Print(Document d);
}
public interface IScanner
{
void Scan(Document d);
}
public interface IFaxer
{
void Fax(Document d);
}
public interface IMultiFunctionDevice : IPrinter, IScanner, IFaxer
{
}
public class Printer : IPrinter
{
public void Print(Document d)
{
// implementation here
}
}
public class Faxer : IFaxer
{
public void Fax(Document d)
{
// implementation here
}
}
public class Photocopier : IPrinter, IScanner
{
public void Print(Document d)
{
// implementation here
}
public void Scan(Document d)
{
// implementation here
}
}
public class MultiFunctionMachine : IMultiFunctionDevice
{
// compose this out of several modules
private IPrinter printer;
private IScanner scanner;
private IFaxer faxer;
public MultiFunctionMachine(IPrinter printer, IScanner scanner, IFaxer faxer)
{
this.printer = printer;
this.scanner = scanner;
this.faxer = faxer;
}
public void Print(Document d) => printer.Print(d);
public void Scan(Document d) => scanner.Scan(d);
public void Fax(Document d) => faxer.Fax(d);
}
The idea here is to split related functionality of a complicated interface into separate interfaces so as to avoid forcing clients to implement functionality that they do not really need. Whenever you find yourself writing a plugin for some complicated application and you’re given an interface with 20 confusing methods to implement with various no-ops and return nulls, more likely than not the API authors have violated the ISP.
Dependency Inversion Principle
A class should depend on abstractions rather than concretions, meaning that it should rely on interfaces or abstract classes rather than concrete classes.
Violating DIP
public enum Relationship
{
Parent,
Child,
Sibling
}
public class Person
{
public string Name;
// DoB and other useful properties here
}
public class Relationships // low-level
{
public List<(Person, Relationship, Person)> Relations = new();
public void AddParentAndChild(Person parent, Person child)
{
Relations.Add((parent, Relationship.Parent, child));
Relations.Add((child, Relationship.Child, parent));
}
}
public class Research
{
public Research(Relationships relationships)
{
// high-level: find all of john's children
var relations = relationships.Relations;
foreach (var r in relations
.Where(x => x.Item1.Name == "John"
&& x.Item2 == Relationship.Parent))
{
Console.WriteLine($"John has a child called {r.Item3.Name}");
}
}
}
The approach illustrated here directly violates the DIP because a high-level module Research directly depends on the low- level module Relationships. Why is this bad? Because Research depends directly on the data storage implementation of Relationships: you can see it iterating the list of tuples. What if you wanted to later change the underlying storage of Relationships, perhaps by moving it from a list of tuples to a proper database? Well, you couldn’t, because you have high-level modules depending on it.
DIP
public enum Relationship
{
Parent,
Child,
Sibling
}
public class Person
{
public string Name;
// DoB and other useful properties here
}
public interface IRelationshipBrowser
{
IEnumerable<Person> FindAllChildrenOf(string name);
}
public class Relationships : IRelationshipBrowser // low-level
{
// no longer public!
private List<(Person, Relationship, Person)> relations = new();
public IEnumerable<Person> FindAllChildrenOf(string name)
{
return relations
.Where(x => x.Item1.Name == name
&& x.Item2 == Relationship.Parent)
.Select(r => r.Item3);
}
}
public class Research
{
public Research(IRelationshipBrowser browser)
{
foreach (var p in browser.FindAllChildrenOf("John"))
{
Console.WriteLine($"John has a child called {p.Name}");
}
}
}
Please note that the DIP isn’t the equivalent of dependency injection (DI), which is another important topic in its own right. DI can facilitate the application of the DIP by simplifying the representation of dependencies, but those two are separate concepts.