Sometimes you’ll find a method on your utility class that’s only used once - or only from a single consuming class. This frequently happens when a developer genuinely believes the method will be generally useful and should be available for reuse, but is wrong.

I’ve seen this happen many times. I’ve done this many times.

However, I’ve found my strike rate to be regrettably low - in most cases, the method isn’t generally useful at all, doesn’t end up being reused by anyone (not even me), and doesn’t need to be kept somewhere public for reuse.

Sometimes this happens because methods dumped into a random utility class have poor discoverability. Due to their grab-bag nature, utility classes have low cohesion, and this makes it challenging for a developer to predict when it’s useful to check the utility class for something useful.

Consider this method, used to obtain all the types declared within an assembly:

public static IEnumerable<Type> AssemblyLoadableTypes(Assembly assembly)
{
    assembly.MustNotBeNull(nameof(assembly));

    try
    {
        return assembly.GetTypes();
    }
    catch (ReflectionTypeLoadException e)
    {
        return e.Types.Where(t => t != null);
    }
}

Some background: if one of the declared types in an assembly has a dependency that is not satisfied, the call to .GetTypes() will fail, throwing a ReflectionTypeLoadException. That exception contains all of the types that could be fully loaded. In the context of this particular project, that’s a good enough list to use.

We could create an AssemblyExtensions static class and relocate the method (as suggested in Killing the utility class with extension methods:

public static IEnumerable<Type> LoadableTypes(this Assembly assembly)
{ }

However, this method is only called from one place - and the details of its behavior are specific enough that it’s unlikely to be reused anywhere else.

Keeping this around for general reuse doesn’t make sense.

Instead, we should this into a private method on the consuming class:

private IEnumerable<Type> FindLoadableTypes(Assembly assembly)
{ }

The key here is to recognize that the method is not actually well suited for reuse. In the example, this is because the method has specific behaviour that only makes sense in one context. Another indicator is when you find a method that’s been in the codebase for several years and still has just one consumer.

An orphan method found in your utility class might have been intended to be reusable … but when it’s not actually being reused, you should dig into why. This might be because no one else has found it (as mentioned above, utility classes are terrible for discovery), in which case you should make it more discoverable by moving it. Or, if the method is actually very specific to a particular use case and not suitable for reuse, move it closer to where it’s used, make it private, and move on.

Comments

blog comments powered by Disqus