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.
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.
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.
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:
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:
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.
There are two SOLID principles that should be your yardsticks here: The Open/Closed Principle, and Dependency Inversion.
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.
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: ""))
Keep shipping your Ruby and JavaScript apps fast and tackle tech debt before it hurts.