In this article, we are going to discover which anti-patterns fall into the class of code smells known as change preventers, as well as several approaches for refactoring them in C#.
Let’s dive in!
What Are Change Preventers?
Change preventers are pieces of code that are constructed in such a manner that changing them necessitates modifications to many other areas of the program. As a result, changing code becomes more complicated and time-consuming.
In this group, we can distinguish three anti-patterns:
- Divergent change
- Shotgun surgery
- Parallel inheritance hierarchies
Let’s look at how to detect them in the code and how to fix them!
Divergent Change
The code smell refers to a situation in which one class can be frequently altered in various ways for various reasons. This is directly related to the Single Responsibility Principle violation. As the definition of this principle says, classes should have only one responsibility and one reason to change.
Let’s go straight to an example. We have a class called Patient
with the method ExportTreatmentHistory()
, which generates a CSV file with all past illnesses of the patient:
public class Patient { public string FirstName { get; private set; } public string LastName { get; private set; } public List<string> TreatmentHistory { get; private set; } public Patient(string firstName, string lastName) { FirstName = firstName; LastName = lastName; TreatmentHistory = new List<string>(); } public void AddTreatment(string treatment) { TreatmentHistory.Add(treatment); } public void ExportTreatmentHistory(string fileName) { using var writer = new StreamWriter(fileName); foreach (var treatment in TreatmentHistory) { writer.WriteLine(treatment); } } }
After we ship this code, our client may ask us to add two functionalities to our system. Firstly, add the insurance number of a patient. Then add the possibility to generate files containing treatment history but as JSON. For both tasks, we have to modify the Patient
class, therefore we can easily see that a class has two responsibilities and two reasons to change it.
The correct refactoring of this problem is to extract a new class so that each of them does only one thing. For this purpose, we will create the Patient
class containing only the patient’s data and the methods needed to modify them:
public class Patient { public string FirstName { get; private set; } public string LastName { get; private set; } public string InsuranceNumber { get; private set; } public List<string> TreatmentHistory { get; private set; } public Patient(string firstName, string lastName, string insuranceNumber) { FirstName = firstName; LastName = lastName; InsuranceNumber = insuranceNumber; TreatmentHistory = new List<string>(); } public void AddTreatment(string treatment) { TreatmentHistory.Add(treatment); } }
Now, to handle the export of the treatment history we could create a class called TreatmentHistoryExporter
and place both methods for exporting there, but we can do something better!
Let’s create an ITreatmentHistoryExporter
interface:
public interface ITreatmentHistoryExporter { void ExportTreatmentHistory(string fileName, List<string> treatments); }
Now we can add two classes that both implement the interface and call them CsvTreatmentHistoryExporter
and JsonTreatmentHistoryExporter
:
public class CsvTreatmentHistoryExporter : ITreatmentHistoryExporter { public void ExportTreatmentHistory(string fileName, List<string> treatments) { using var writer = new StreamWriter(fileName); foreach (var treatment in treatments) { writer.WriteLine(treatment); } } } public class JsonTreatmentHistoryExporter : ITreatmentHistoryExporter { public void ExportTreatmentHistory(string fileName, List<string> treatments) { var json = JsonSerializer.Serialize(treatments); using var writer = new StreamWriter(fileName); writer.Write(json); } }
In this case, each class is responsible for exactly one thing, so modifications become easier – we have exactly one reason to modify each of them.
Shotgun Surgery
Let’s start right away with an example to explain what this code smell actually is. First, we need to assume that we have a banking application in which we want to know who performed a given action and also be able to present the entire history of operations for a given user.
For this reason, we create a class called Action
, containing fields such as ActionName
, UserId
:
public class Action { public string ActionName { get; set; } public int UserId { get; set; } }
Each user has a list of performed actions, which is updated with each operation:
public class User { public int Id { get; set; } public string FirstName { get; set; } public string LastName { get; set; } public List<Action> Actions { get; set; } public void ModifyFirstName(string firstName) { FirstName = firstName; var action = new Action { ActionName = nameof(ModifyFirstName), UserId = Id, }; Actions.Add(action); } public void ModifyLastName(string lastName) { LastName = lastName; var action = new Action { ActionName = nameof(ModifyLastName), UserId = Id, }; Actions.Add(action); } }
In each application, the number of possible operations for a given user is very large. So what happens if we are asked to add a field containing the date and time of the operation to the Action
class? Every place in which we can add a new action to the list will need an update.
This is an example of the Shotgun Surgery code smell. Making one modification requires us to change many places. Often this change will be very similar or will require us to copy the code to multiple places. A red light should go on! What if we forget to add it somewhere?
To deal with this, we need to do as with any duplicate code – move the common part to a new method so that in the future we only have one place where we will make possible modifications.
So, let’s add a new field to the Action
class:
public class Action { public string ActionName { get; set; } public int UserId { get; set; } public DateTime CreatedAt { get; set; } }
And now we’ll see what the User
class will look like after creating a separate method for modifying the Actions
list:
public class User { public int Id { get; set; } public string FirstName { get; set; } public string LastName { get; set; } public List<Action> Actions { get; set; } public void ModifyFirstName(string firstName) { FirstName = firstName; AddAction(nameof(ModifyFirstName)); } public void ModifyLastName(string lastName) { LastName = lastName; AddAction(nameof(ModifyLastName)); } private void AddAction(string actionName) { var action = new Action { ActionName = actionName, UserId = Id, CreatedAt = DateTime.Now }; Actions.Add(action); } }
If further modifications are needed, we can do everything in the AddAction
method. It will be faster, easier, and safer.
Parallel Inheritance Hierarchies as Change Preventer
Simply put, this problem occurs when creating a new class forces us to add one more class. When one of the classes depends on the other, this situation arises. Parallel Inheritance Hierarchies cause a lot of superfluous classes and make the code very brittle and tightly coupled.
Let’s go straight to an example.
First, we are going to create an abstract Teacher
class:
public abstract class Teacher { public abstract string ShowCurriculum(); }
Each teacher has their own Curriculum
:
public abstract class Curriculum { public abstract string GetPlannedEducationalOutcomes(); }
Now is the time to add a class for a teacher that teaches a language:
public class LanguageTeacher : Teacher { public override string ShowCurriculum() { return new LanguageCurriculum().GetPlannedEducationalOutcomes(); } }
LanguageCurriculum
should contain all the planned educational outcomes:
public class LanguageCurriculum : Curriculum { public override string GetPlannedEducationalOutcomes() { return "Educational outcomes for language learning"; } }
As we already know, there are differences between what we learn in language classes and what we learn in physical education classes, hence PE teachers must follow a different Curriculum
:
public class PhysicalEducationTeacher : Teacher { public override string ShowCurriculum() { return new PhysicalEducationCurriculum().GetPlannedEducationalOutcomes(); } }
Now, it is clear that adding a new class to the hierarchy of curricula is necessary if we want to include a new type of teacher. Because of this coupling, the maintenance of the code can be also harder, as the change in one class may force us to change the second as well.
Refactoring Parallel Inheritance Hierarchies
How can we refactor this code smell?
Depending on our specific situation, we may eliminate this code smell through a variety of methods. The easiest way is to only have one hierarchy that implements details for both the teacher and the curriculum.
Because it breaks Single Responsibility Principle, it isn’t always worthwhile. Sometimes it makes more sense to accept this code smell than to search for other solutions. On the other hand, assuming we have multiple types of teachers, and the curriculum class only contains a method that returns the planned outcome, we can get rid of the extra indirection, for example in the LanguageTeacher
class:
public class LanguageTeacher : Teacher { public override string ShowCurriculum() { return "Educational outcomes for language learning"; } }
Conclusion
In this article, we discussed the popular code smells from the change preventers group and learned how to deal with them. Avoiding these antipatterns will help you get better code organization, less code duplication, and easier maintenance.