One of the lesser known code smells, Divergent Change is one of the strongest multipliers of tech debt. Fixing it has high leverage, so we'll explore how to spot and remediate it.
What Is Divergent Change?
Divergent Change belongs to the code smells class of Change Preventers. As such, it is a major impediment for your development velocity, which is why it should be treated swiftly when you spot it.
In one sentence, you are likely dealing with this smell if one class changes often, but for unrelated reasons. Another hint is the necessity to change a class in multiple places when altering it or a dependency. It is the mirror-universe counterpart of Shotgun Surgery, in which many classes change for the same reason.
I am going to demonstrate this with an Alert ViewComponent:
class AlertComponent < ViewComponent::Base
def initialize(state:, message:)
@state = state
@message = message
end
def color
{
success: "green",
info: "blue",
error: "red"
}
end
def icon
{
success: "check",
info: "info",
error: "exclamation"
}
end
def message
prefix = case state
when :success then "YIPPEE!"
when :info then "Listen up:"
when :error then "Oh Dear."
end
"#{prefix} #{@message}"
end
end
Suppose we want to add a new type of status, warning, we now have to edit three individual methods. This example is trivial, but imagine these methods are actually spread out between several modules. Suddenly a single change turns into a bug chasing contest.
What Makes Divergent Change So Harmful?
Every "change preventer" is malign, but Divergent Change is often harder to spot than its colleagues. That's especially the case if a class' behavior is scattered between various modules, for example Rails' ActiveSupport concerns.
That's why extracting a concern solely to include it in a single class - albeit reducing perceived complexity - is itself considered a smell. It disguises that a class doesn't follow the Single Responsibility Principle, and should be better treated with other techniques, for example composition.
Often, divergent change starts out pretty innocent: "Oh, I'll just use a hash as a lookup table", or "one case statement won't hurt". A simple enough choice in the YAGNI spirit - fair enough. The problem is one of setting an example, i.e. culture. The next developer who needs to program a feature in this class sees the pattern and does the next best thing under deadline pressure - add another conditional. Thus, gradually the class becomes a God Class with far too many concerns mixed into its responsibility - another way of circumscribing Divergent Change. What's worse, such a static structure of conditionals and constants makes it hard to refactor and employ a behavioral design pattern like Strategy or Visitor, which allow to dynamically attach behavior at runtime.
How Do I Detect Divergent Change in My Codebase?
In contrast to Shotgun Surgery, which is easier to spot (you just have to grep a certain method or class in your codebase), Divergent Change is a master of camouflage. Chances are you already have many such offenses in your codebase which are hiding in plain sight. Let's look at a strategy to detect it by first narrowing down the potential offenders, then looking for specific symptoms.
Conveniently, Attractor already equips you with the relevant code metrics to filter your classes in search of Divergent Change:
- Churn (how often a class has changed)
- Complexity (how hard it is to understand what the class is doing)
If you restrict your search to those files exhibiting both a high churn count and complexity, you are likely to come across a couple of God Classes that display the symptoms outlined above.
That's of course only narrowing down the search space. We have to do a bit of detective work to tease out the offenses. Here are a few indicators that you should be familiar with:
- Looking through the version control log, you see many changes that aren't related to each other
- If you skim through these commits, you notice that more than one class or module was modified
- The class has a lot of control flow logic (i.e. conditionals)
How Do I Treat Divergent Change?
As a general heuristic, you should strive that each class only serves one purpose. Ideally you can describe what it does in a single sentence. That, of course, is only a very general guideline.
I will start out with a simple refactoring that introduces composition by extracting a class (hierarchy). Bear with me, I'm deliberately taking this step by step.
class Alert
def initialize(message:)
@message = message
end
end
class SuccessAlert < Alert
def color = "green"
def icon = "check"
def message = "YIPPEE! #{@message}"
end
class InfoAlert < Alert
def color = "blue"
def icon = "info"
def message = "Listen up: #{@message}"
end
class ErrorAlert < Alert
def color = "red"
def icon = "exclamation"
def message = "Oh Dear. #{@message}"
end
class AlertComponent < ViewComponent::Base
VALID_ALERTS_BY_STATE = {
success: SuccessAlert,
info: InfoAlert,
error: ErrorAlert
}
def initialize(state:, message:)
@state = VALID_ALERTS_BY_STATE[state].new(message: message)
end
delegate :color, :icon, :message, to: :state
end
Note how this simple refactoring has already reduced the changes that have to be made to add a new type of alert from 3 to 2: Instead of altering 3 methods, I now only have to add one class, and add it to the VALID_ALERTS_BY_STATE hash.
We can do better though. We harness Rails' developers dearest paradigm - Convention over Configuration - to simplify the class lookup:
class AlertComponent < ViewComponent::Base
def initialize(state:, message:)
@state = "#{state.classify}Alert".safe_constantize.new(message: message)
end
delegate :color, :icon, :message, to: :state
end
Now we've also gotten rid of the lookup hash and simply assume that the class name adheres to a certain naming scheme. Note that both these approaches still require that you know which are valid states to pass into the component. As we will see in a moment, there's a yet better way to go about this.
How Can I Prevent It?
There are two SOLID principles that should be your yardsticks here: The Open/Closed Principle, and Dependency Inversion.
Open/Closed Principle
Paraphrased a bit, it goes like this:
Software entities should be open for extension, but closed for modification.
We have seen in the above example that adding a new type of alert would have meant tinkering with the internals of AlertComponent quite a bit. You should always strive to see classes as "finished" entities and not interfere with their implementation if possible. Of course, this requires discipline and often quick iteration in the product development lifecycle exacerbates the situation.
With refactorings like the above we have already improved the situation. Still, there's a bit of concern regarding the alert class lookup: If the naming convention was ever to change, we'll have to touch a lot of classes, again. It would be better if the AlertComponent didn't have to bother with that at all and were extensible by design. This is where the next principle comes in.
Dependency Inversion
In short, this principle demands that the software inverts control. Lower level modules (our Alert classes) should be passed into higher level modules (our AlertComponent) instead of the latter depending on the former. Rather, they should be treated as abstractions. What does this mean? Let's take a look:
class AlertComponent < ViewComponent::Base
def initialize(alert:)
@alert = alert
end
delegate :color, :icon, :message, to: :alert
end
Now the AlertComponent class is completely decoupled. We have inverted control, i.e. the component simply relies upon the passed in alert to understand color, icon, and message. It would be called like this:
render AlertComponent.new(alert: SuccessAlert.new(message: "You have successfully inverted control"))
Notably, the component now isn't concerned with adding a new type of alert at all anymore. All we'd have to do is implement it as a class and inject it:
class WarningAlert < Alert
def color = "yellow"
def icon = "triangle-exclamation"
def message = "Yikes. #{@message}"
end
render AlertComponent.new(alert: WarningAlert.new(message: ""))
Top comments (0)