The Cost Of Changing Code
It is desirable to distribute the responsibilities among classes well by keeping the cost of maintenance and adaptation of a code base under control. The more responsibilities a class has, the more reasons it has to change, and changing code has a cost in terms of development time spent reading, understanding, and finally (hopefully) iteratively changing its tests and the code itself. With change, a certain risk of regression or bugs that can slip through the review process always comes as well.
Then, what can we do to reduce the cost of software evolution? We can continuously review the responsibilities of our classes and segregate them when necessary, modularize the code to the best of our knowledge such that the reasons for a class or a module to change will be minimal.
In this perspective, we are pursuing a code design that is open to extension and close to change (sounds familiar?). In the following article, and the next articles to come, I aim to collect a few patterns and use cases I often see during code reviews and propose solutions that should help make the code more open for extension and tests easier to maintain.
Use cases
1. I want to add a conditional behavior behind the feature toggle to an existing component
Let's imagine that we find ourselves in need of transitioning from one implementation of a service provider to another, for example from a push notification provider to another, and have to gradually move the messages. The starting code would be:
class OrderAcceptedSubscriber(val notifier: Notifier) {
fun onEvent(event: OrderAccepted) {
// ...
notifier.notify(Message(...))
}
}
class OrderDeliveredSubscriber(val notifier: Notifier) {
fun onEvent(event: OrderDelivered) {
// ...
notifier.notify(Message(...))
}
}
A naive implementation would add a condition everywhere to call the new implementation behind a feature toggle for each call site, like this:
class OrderAcceptedSubscriber(
val notifier: Notifier,
val newNotifier: Notifier,
val toggles: FeatureToggles) {
fun onEvent(event: OrderAccepted) {
// ...
if (featureToggles.isNewNotifierEnabledForOrderAccepted()) {
newNotifier.notify(Message(...))
} else {
notifier.notify(Message(...))
}
}
}
class OrderDeliveredSubscriber(
val notifier: Notifier,
val newNotifier: Notifier,
val toggles: FeatureToggles) {
fun onEvent(event: OrderDelivered) {
// ...
if (featureToggles.isNewNotifierEnabledForOrderDelivered()) {
newNotifier.notify(Message(...))
} else {
notifier.notify(Message(...))
}
}
}
The problem with this approach is the increasing cost of implementation and maintenance: for each site where a notification is sent, we will have to change the code to add the new Notifier call, change the tests to consider the toggle interaction and a new case, and likely create a new toggle for each notification. The cost of change amplifies as new notifications are migrated and again when the old Notifier implementation is removed (this last point is often neglected in the phase of development). It's worth noticing the tight coupling now between the subscriber and the toggle – the subscriber has a static dependency on that specific toggle method.
If we favored extensibility and maintainability over direct implementation, we could decorate the original Notifier and create a role FeatureToggle that can be reused. The following code is an example:
class OrderAcceptedSubscriber(val notifier: Notifier) {
fun onEvent(event: OrderAccepted) {
// ...
notifier.notify(Message(...))
}
}
class OrderDeliveredSubscriber(val notifier: Notifier) {
fun onEvent(event: OrderDelivered) {
// ...
notifier.notify(Message(...))
}
}
class MigrationNotifier(
val toggle: FeatureToggle,
val legacyNotifier: Notifier,
val newNotifier: Notifier) : Notifier {
fun notify(msg: Message) {
if (!toggle.isEnabled())
legacyNotifier.notify(msg)
else
newNotifier.notify(msg)
}
}
class HttpFeatureToggle(
val name: String,
val client: HttpClient) : FeatureToggle { ... }
And wire the proper toggle within the MigrationNotifier for each use case like:
val orderAcceptedNotifier = MigrationNotifier(
HttpFeatureToggle("IS_ORDER_ACCEPTED_ENABLED", client),
legacyNotifier,
newNotifier)
val orderDeliveredNotifier = MigrationNotifier(
HttpFeatureToggle("IS_ORDER_DELIVERED_ENABLED", client),
legacyNotifier,
newNotifier)
It would be even possible to dynamically construct the toggle out of some properties of the message itself being sent if desired, effectively removing the necessity of multiple toggles and MigrationNotifier instances.
class HttpMessageFeatureToggle(
val name: String,
val client: HttpClient) : FeatureToggle {
fun isEnabledFor(msg: Message) {
val toggleName = "IS_ENABLED_FOR_${msg.name()}"
// ...
}
}
Notice how we take advantage of OO and encapsulate state (toggle name) and behavior (http call) by explicitly representing the concept of feature toggle as HttpFeatureToggle, making it possible to reuse the code and the toggle instance in multiple places if necessary.
The difference between the two approaches is that in the second one, the design is extended – new code is added rather than being modified, and at the moment that the legacy implementation is dismissible, no code will be modified if not the one to glue the components together: we would have to replace orderAcceptedNotifier and orderDeliveredNotifier with newNotifier, and finally only delete the old and the migration code.
The implementation of this last approach is not always this straightforward, at times we might find that the component we want to decorate is leaking implementation details and the contract doesn't get along with the interface of the new provider. In this case, we should introduce a new abstraction first (Notifier in this example) and then implement the decorator. But, wouldn't that cost the same as adding the if in every use case? No, because with the proper abstraction, as we were saying before, we will reduce remarkably the cost of change when the legacy implementation is removed or likely in further iterations where we might want to modify the implementation for any reasons – like adding observability, monitoring or caching, and this brings us to the next example.
2. I have to always do something before, after, or around a component call
Now that we tackled the benefits of a more extensible approach above, I won't indulge anymore in the reasons but rather dive straight into the examples.
In this case, I always need to do something before and after a call to a collaborator, that is, there is a temporal coupling between two or more actions. One always has to happen before the other, otherwise bad things will happen. A typical example of this is tracking statistics, let's see the code we want to change:
class SqlOrdersRepository(...) : OrdersRepository {
fun allIn(ids: List<Long>): List<Order> {
// select
}
}
I might be tempted to add the collection and tracking of statistics there, where the call I want to track is happening, where the data is!
class SqlOrdersRepository(...) : OrdersRepository {
fun allIn(ids: List<Long>): List<Order> {
val startTimeMs = System.currentMills()
val result = ... // select
val durationMs = System.currentMills() - startTimeMs
val numOfIds = ids.size()
metrics.count("allIn.params", numOfIds)
metrics.count("allIn.durationMs", durationMs)
return result
}
}
The downside of this approach is that the component gained another responsibility (metric tracking). As a result, there are more reasons for this class to change, and the same for the tests which became more complex too (because of the metrics interactions, and we're not even considering the error cases). In the unfortunate case where we might want to change the implementation of the repository (e.g. from JDBC to JPA), we will have to either reimplement the tracking of the statistics or the repository itself. And what if we decide later that we want to measure stats after caching?
Again, the decorator comes to help in this case:
class SqlOrdersRepository(val jdbc: JdbcTemplate) : OrdersRepository {
fun allIn(ids: List<Long>): List<Order> {
// select
}
}
class OrdersRepositoryStats(
val delegate: OrdersRepository,
val metrics: Metrics) : OrdersRepository {
fun allIn(ids: List<Long>): List<Order> {
val startTimeMs = System.currentMills()
return delegate.allIn(ids).also {
val durationMs = System.currentMills() - startTimeMs
val numOfIds = ids.size()
metrics.count("allIn.params", numOfIds)
metrics.count("allIn.durationMs", durationMs)
}
}
}
class Config {
fun ordersRepository(metrics: Metrics, jdbc: JdbcTemplate): OrdersRepository {
return OrdersRepositoryStats(SqlOrdersRepository(jdbc), metrics)
}
}
The delegate can be any implementation, even another decorator. This time as well, if the statistics are no longer necessary, the change would be as simple as deleting the class and its tests, with no need to modify anything else except the code that glues the components together (e.g. Spring config).
3. I want to make sure no exception breaks the flow
Sometimes, there might be multiple components participating in the carrying on of an action that implements the same interface (command, a request, or processing an event). One good example of such a case is a Composite, where a certain message (meant like a method call) is propagated to each component and it's undesirable to stop scrolling through them or bubble up an error if one of those suddenly breaks, raising an exception. In such a case, to continue the processing of the request in the event of an error, what we can do is then wrap each component in a catch-all decorator:
class CourierPositionController(val observer: PositionObserver) {
fun newPosition(courierId: CourierId, coord: Coordinates) {
// ...
observer.on(Move(courierId, coords))
}
}
class AllPositionObservers(vararg observers: PositionObserver) : PositionObserver {
init {
val observers = observers.map(o -> SafePositionObserver(o))
}
override fun on(event: Move) {
observers.forEach(o -> o.on(event))
}
}
class SafePositionObserver(val delegate: PositionObserver) : PositionObserver {
fun on(event: Move) {
try {
delegate.on(event)
} catch (e: Exception) {
// log
// metric
}
}
}
4. I want to cache an object coming from a component
If there's the need to cache a collaborator's return value, then a decorator is a match made in heaven. The consumers of the API being cached won't see any differences (if the abstraction is well-designed) and for them, it doesn't matter if the component is cached or not. The tests for both the cache and the cached component will be simpler than if it was a single class with both responsibilities, which is often good feedback about our design:
class HttpPhoneRepository(...) : PhoneRepository { ... }
class CachingPhoneRepository(val cache: Cache, val delegate: PhoneRepository) : PhoneRepository {
override phoneOf(userId: UserId): PhoneNumber {
val valueLoaderOnMiss = { uid -> delegate.phoneOf(uid) }
return cache.get(userId, valueLoaderOnMiss)
}
}
5. I want to quickly experiment with or debug something and remove it right after
In this case, I'd like to run a quick, small experiment to get feedback on a new idea, for example, add a few new fields to a json response which will trigger a new feature (driven by the FE). Instead of changing the code to add the new field mappings and changing the code again later on to remove them, I might decorate the mapper and delete the class once I am done with the experiment (perhaps once I know more about what we want to do and how to do it).
class DefaultOrderDetailResponseMapper : OrderDetailResponseMapper {
override fun map(order: Order): OrderDetailResponse {
val resp = OrderDetailResponse()
// ...
return resp
}
}
class LowerDeliveryTimeMapper(val delegate: OrderDetailResponseMapper) : OrderDetailResponseMapper {
override fun map(order: Order): OrderDetailResponse {
val result = delegate.map(order)
if (result.id % 42 == 0) {
result.deliveryTimeInSecs -= 300
}
return result
}
}
There is a better alternative approach to do the same thing – especially for mappers/adapters when there are multiple actors interested in mutating the response – which I will explain in one of the next articles.
Use with Caution
A word of caution, too many wrapping classes can make the logic harder to visualize and subsequently, to understand. That might be a symptom of the overuse of decorators. In that case, sometimes we might want to revisit our design and redistribute responsibilities, maybe even aggregate them in a single role (we will see alternatives in the next articles) In the end, it's always a matter of which trade-off to choose.
Conclusions
I hope this article was helpful for some of you and that you liked it. If you can think of other or better examples to add to the list or have any feedback at all, let me know or drop them in the comments! Stay tuned for more, I'll share some thoughts about other patterns and use cases in the coming articles.
Top comments (0)