The Problem With Helpers, Manager and Utils
Have you ever come across a class in a project you’ve worked on with something like “helper”, “service” or “util” in the name? Unless you’re one of the fortunate few who’ve never had to regularly have a steaming hot plate of spaghetti code for your breakfast, lunch and dinner, then I’m guessing you have.
Do you remember if that file was easy to work with? I bet it wasn’t. Was it easy to change? Was it stealthily coupled to all the things? And, did it have more than one responsibility? These are the things I generally find with such classes. They’re a confusing swamp of incohesive code that’s used in all sorts of places. Want to refactor it? Put on your best slacks and get comfy, you’ll be there for a while.
Swamp Thing, You Make My Code Stink
The thing with the humble helper is that ironically, it’s anything but helpful. A helper class usually just ends up full of globally accessible methods which create silent dependencies in your project, and they usually don’t represent just one concept in your domain. Effectively they end up being used as more of a namespace for a loosely connected group of methods more than a class which will be used to instantiate true objects.
When you look at one of these sorts of classes I think the first questions should be, what does this actually represent, what does it model? Secondly, seeing as it’ll be a static class (in the C# world), it won’t be encapsulating anything and it’ll also have logic in it so it’s more than just a value class in the DDD sense. So what are they? We can’t use them like objects that do something useful by encapsulating other collaborators, and we can’t inject them into other objects either. They make me think of this guy…
Swamp Thing classes are what they are, and in the image above from left to right, we have Util, Impl, Helper and Service. The Swap Class Crew.
I’ve seen these poisonous beasties used in views, and that’s just not cool man. And that’s the thing, these helpers and utils are purposely made to be easily accessible from anywhere, even a view! If you have one or more of these types of class in your project and they’re not kept under control they get their tendrils into every layer of your application. All aboard the Helper Express, stopping at Leaky Abstractionsville, Big Ball of Muddington and Developer Misery Central.
Another thing that seems to go hand in hand with them too is the long parameter list code smell. This seems to be a by-product of the fact these guys can be called from anywhere, and that over time due to being used in many different contexts the parameter list grows to accommodate that extra bit of state the calling context needs it to know about to do its job.
What we can say about these types of classes then is that:
- They make testing difficult
- They’re namespaces for procedural code and promote procedural thinking
- They don’t represent a single concept in your domain
- They potentially have more than one responsibility
- They create silent dependencies
So what can we do about them?
Science Transformed Him into a Monster, Love Changed Him Even More
When I think of helpers and the like, for whatever reason I always think of them like mango or an orange that has been turned inside out:
They’re one or more hidden types that are spewing out their guts for all to see. Maybe the types already exist and the logic just needs moving onto them, either way, the logic should be refactored so that it’s properly encapsulated. Below I’ve written a little helper that stinks more than a block of Stilton that’s been left to fester in the baking hot summer Sun:
public static class ProductHelper
{
public static bool IsEligibleForFreeShipping(decimal productCost)
=> productCost > 50m;
public static decimal ApplyOfferDiscount(decimal productCost)
=> DateTime.Now.DayOfWeek.Equals(DayOfWeek.Tuesday)
? productCost / 2
: productCost;
public static string CatalogId(string productId, string manufacturerId)
=> $"{productId}-{manufacturerId}";
}
This class is nuts for all sorts of reasons:
- Using primitive types to express complex concepts
- Multiple concerns, cost calculation, discount calculation and formatting
- Low cohesion, when will it stop growing?
- Magic values, 50m, 2
- Tests for the discount method will fail 6 days of the week
How do we then make this a little more cohesive? Does it look like there is a Product type hidden away in there perhaps? Lots of primitives are being passed around to express the concept of a product, and then we have some class manipulating that data, let’s just move it onto a Product type:
public class Product
{
public string Id { get; }
public string ManufacturerId { get; }
public decimal Cost { get; }
public Product(string id, string manufacturerId, decimal cost)
{
Id = id;
ManufacturerId = manufacturerId;
Cost = cost;
}
}
A Product type has now been extracted so the next step is to refactor the helper class to use the type instead of primitives:
public static class ProductHelper
{
public static bool IsEligibleForFreeShipping(Product product)
=> product.Cost > 50m;
public static decimal ApplyOfferDiscount(Product product)
=> DateTime.Now.DayOfWeek.Equals(DayOfWeek.Tuesday)
? product.Cost / 2
: product.Cost;
public static string CatalogId(Product product)
=> $"{product.Id}-{product.ManufacturerId}";
}
That’s a little more expressive but now there’s a big waft of the feature envy code smell drifting through the air. To sort that out we can move the envied state/behaviour onto the envied type. If you’re a C# developer using ReSharper you can use the shortcut CTRL + R + O. Do this while the cursor is somewhere in a methods name to move it to another type. Time to move some methods:
public class Product
{
private readonly string _id;
private readonly string _manufacturerId;
private readonly decimal _cost;
public Product(string id, string manufacturerId, decimal cost)
{
_id = id;
_manufacturerId = manufacturerId;
_cost = cost;
}
public string CatalogId()
=> $"{_id}-{_manufacturerId}";
public bool IsEligibleForFreeShipping()
=> _cost > 50m;
public decimal ApplyOfferDiscount()
=> DateTime.Now.DayOfWeek.Equals(DayOfWeek.Tuesday)
? _cost / 2
: _cost;
}
The methods have now been moved off the helper and onto Product. However, our tests will still be failing if it’s not Tuesday due to the dependency on DateTime. We can fix that by pushing the dependency out moving DateTime to be a constructor parameter. With ReSharper, you can do this by highlighting DateTime.Now and using the CTRL + R + F shortcut to extract a field. It will ask you where you’d like to initialize the field, choose “initialize from constructor”. Then highlight the right side of the subsequent field’s assignment and use CTRL + R + P to extract it to a constructor parameter. That will leave us with something like this:
public class Product
{
private readonly string _id;
private readonly string _manufacturerId;
private readonly decimal _cost;
private DateTime _dateTime;
public Product(string id, string manufacturerId, decimal cost, DateTime dateTime)
{
_id = id;
_manufacturerId = manufacturerId;
_cost = cost;
_dateTime = dateTime;
}
public string CatalogId()
=> $"{_id}-{_manufacturerId}";
public bool IsEligibleForFreeShipping()
=> _cost > 50m;
public decimal ApplyOfferDiscount()
=> _dateTime.DayOfWeek.Equals(DayOfWeek.Tuesday)
? _cost / 2
: _cost;
}
Now whilst the Product class still smells it is at least something that has state and behaviour. It can be called an object and the helper class is no more. We can also inject a date to test the ApplyOfferDiscount method. However the name doesn’t really make sense, and it maybe it shouldn’t even be on Product!
You can refactor helpers, utils and those ill-conceived impl and service classes. With a bit of love, they can be refactored to types that represent concepts in your problem domain. Go slay some “helpers”, happy refactoring!
The follow-up to this explores some of the learnings from Adam Tornhill’s fantastic book, “Your Code as a Crime Scene”. It explains how we can use what’s explained in the book to see the impact that helpers, managers and utils have on our code. You can find it here.
