Sometimes your utility class will contain methods that smell strongly of feature envy, prompting you to relocate them onto an existing class. This is a much simpler cleanup than introducing a semantic type.

This happens when new methods are added to the utility class instead of being added directly to the class itself. There are many reasons why this might be the case - I’ve seen it happen as a matter of convenience when the target class is in a different repository entirely, and as a side effect of prototyping when the need for the method hasn’t yet been well established.

I have a hunch that this happens more regularly on teams with really strong code ownership, triggered when the official owner of the class is unavailable, or unwilling, to review and accept the suggested change. Often, strong code ownership is a symptom of other issues within the team, but that’s a subject for another post.

What if we found the following methods in our Utility class?

public class Utilities
{
    public static bool SeriesHasId(TimeSeries series, SeriesId id) 
        => series.SeriesId.Equals(id);

    public static bool SeriesHasId(TimeSeries series, string id)
        => series.SeriesId.Equals(new SeriesId(id));
}

In this case, we’ve got a couple of simple methods that test a TimeSeries to see if it has a specified identifier.

For the first method, SeriesHasId(TimeSeries, SeriesId) the solution is straightforward - consolidate the method onto the TimeSeries class, with a quick rename on the way:

public class TimeSeries
{
    public bool HasId(SeriesId id)
        => SeriesId.Equals(id);
}

This makes it much easier to discover - IntelliSense will now show the method; developers don’t have to go rummaging around the Utility class on the off-chance that the method they need is there, somewhere.

The second method is a bit more of a challenge. You could move it onto the TimeSeries class as well like this:

public class TimeSeries
{
    public bool HasId(string id)
        => SeriesId.Equals(new SeriesId(id));
}

If the method is expected to be long-lived, this makes sense. But, if the method is only going to be around while you are making the transition to use of SeriesId as a semantic type, you might be better served turning it into an extension method:

public static class TimeSeriesExtensions
{
    public static bool HasId(this TimeSeries series, string id)
        => series.SeriesId.Equals(new SeriesId(id));
}

Which approach you want to take this depends on the context of your specific project. If in doubt, moving it onto the actual class keeps things simple.

Either way, you might want to consider using an [Obsolete] attribute to encourage the transition forward.

Comments

blog comments powered by Disqus