The Single Responsibility Principle explained with lego
tl;dr
The single responsibility principle states that a class should never do more than one thing.
Don't do this:
The ice-cream program
Say we are trying to build an ice-cream program. Exactly what it does is not important, but it will need some logging, database connectivity and stream reading.
Let us define an interface that our program can use for the stream reading:
interface IStreamReader {
IceCreamData[] ReadFromStream(string fileName)
}
Implementing the stream reader
Now, we'll try to implement the IStreamReader:
public class IceCreamStreamReader : IStreamReader {
public IceCreamData[] ReadFromStream(string fileName) {
// --- Open stream ---
var executablePath = Assembly.GetEntryAssembly().Location;
var filePath = Path.Combine(fileName, executablePath)
var stream = File.OpenRead(filePath)
// --- Read from stream ---
IceCreamData[] iceCreams = new IceCreamData[] {};
var array = new byte[300];
var span = new Span<byte>(array);
while(stream.Read(span,0,span.length) > 0) {
iceCreams.push(ParseAsIceCream(span));
}
// --- Validate data ----
foreach(var iceCream in iceCreams) {
if (iceCream.Size > 5 || iceCream.Size < 1) {
throw new Exception("Invalid ice cream size");
}
if (!PossibleTastes.Contains(iceCream.Taste)) {
throw new Exception("Unknown taste");
}
if (iceCream.Payment != 0) {
throw new Exception("Ice cream payment should be 0")
}
}
}
private IceCreamData ParseAsIceCream(Span<byte> span) {
// A b*ttload of parsing code
// ...
}
}
This is a bit of a wall of text. Did this hurt your eyes/brain to read? Did you notice the missing return at the end?
Worse, the class would grow even clumsier if you would need to:
- Sometimes open the stream from memory or a http-connection
- Use different formats for the stream parsing
- Sometimes add on additional validation rules
The Single Responsibility Principle
The root of the issue is that our IceCreamStreamReader has too many responsibilities. It is supposed to:
- Open the stream
- Parse the stream
- Validate the parsed contents
I.e. we thought we were implementing a single nice lego brick:
But in fact this is what we did. Yuck:
Allow me to reiterate the Single Responsiblity Principle:
Each class should have only one reason for existing, one responsibility.
Splitting into smaller classes
Having identified all the things that our class does, let us break it into separate components:
/**
Responsible for chosing which components to use
(different streamOpeners, parsers, etc.)
-
AND responsible for running the entire flow.
*/
public class IceCreamStreamReader : IStreamReader {
public IceCreamData[] ReadFromStream(string fileName) {
const streamOpener = new FileStreamOpener(fileName);
const parser = new StreamV1Parser();
const validator = new IceCreamValidator();
const stream = streamOpener.OpenStream();
const iceCreams = parser.Parse(stream);
foreach(var iceCream in iceCreams) {
const errorMessage = validator.Validate(iceCream);
if (errorMessage != "") {
throw new Exception(errorMessage);
}
}
return iceCreams;
}
}
public class FileStreamOpener {
private string fileName;
public FileStreamOpener(string fileName) {
this.fileName = fileName;
}
public Stream OpenStream() {
var executablePath = Assembly.GetEntryAssembly().Location;
var filePath = Path.Combine(fileName, executablePath)
var stream = File.OpenRead(filePath)
}
}
public class StreamV1Parser {
public IceCreamData[] Parse(Stream stream) {
IceCreamData[] iceCreams = new IceCreamData[] {};
var array = new byte[300];
var span = new Span<byte>(array);
while(stream.Read(span,0,span.length) > 0) {
iceCreams.push(ParseAsIceCream(span));
}
return iceCreams;
}
private IceCreamData ParseAsIceCream(Span<byte> span) {
// A b*ttload of parsing code
}
}
public class IceCreamValidator {
public string Validate(IceCreamData iceCream) {
if (iceCream.Size > 5 || iceCream.Size < 1) {
return "Invalid ice cream size";
}
if (!PossibleTastes.Contains(iceCream.Taste)) {
return "Unknown taste";
}
if (iceCream.Payment != 0) {
return "Ice cream payment should be 0";
}
return "";
}
}
Notice how we moved all the low-level code away from IceCreamStreamReader. This makes it easier to reason about the flow of data in the program.
Going further
Did you notice the "AND" in the comment for the IceCreamStreamReader? It seems like it has two different responsibilities!
First let's make interfaces for our new building blocks:
interface IStreamOpener {
public Stream OpenStream();
}
interface IStreamParser {
public IceCreamData[] Parse(Stream stream);
}
interface IIceCreamValidator {
public string Validate(IceCreamData iceCream);
}
public class FileStreamOpener : IStreamOpener {
// ... as before ...
}
public class StreamV1Parser : IStreamParser {
// ... as before ...
}
public class IceCreamValidator : IIceCreamValidator {
// ... as before ...
}
Now we can split the main class into its two different responsiblities:
/**
Responsible for chosing which components to use
(different streamOpeners, parsers, etc.)
*/
public class IceCreamStreamReader : IStreamReader {
public IceCreamData[] ReadFromStream(string fileName) {
const streamOpener = new FileStreamOpener(fileName);
const parser = new StreamV1Parser();
const validator = new IceCreamValidator();
const iceCreams = StreamReaderRunner.Run(streamOpener, parser, validator);
return iceCreams;
}
}
/**
Responsible for running the entire flow: opening, parsing, and validating.
*/
public static class StreamReaderRunner {
public static IceCreamData[] Run(IStreamOpener opener, IStreamParser parser, IIceCreamValidator validator) {
const stream = opener.OpenStream();
const iceCreams = parser.Parse(stream);
foreach(var iceCream in iceCreams) {
const errorMessage = validator.Validate(iceCream);
if (errorMessage != "") {
throw new Exception(errorMessage);
}
}
return iceCreams;
}
}
Closing notes
In this case it might be reasonable to ask: is it really up to the IceCreamStreamReader to decide upon validation? Or from which network port to load the stream?
Maybe this decision belongs inside the program's code. In this case the interface between the program and the stream reader is no longer correct, and should be expanded.