Get rid of unneeded factories with one weird trick

Object oriented developers love patterns. Sometimes these patterns are useful and make code easier to understand, other times they are just included for the sake of using patterns. The overuse is probably because they have goofy sounding names. One of the patterns that often has the most dubious value is the Factory Pattern, wherein we create a whole different object that can then create the object we actually wanted for us. This pattern does have some merit in that it can aid in making code reusable, or can abstract away some concerns - however, over eager OOP practitioners will sometimes make a factory that effectively just calls a constructor for one type, and takes all the parameters that that constructor needs - which makes it just an annoying piece of indirection.

To counter patterns with goofy names, we must use refactorings with equally goofy names (it's the only way). So, to aid in removing pointless factories we have:

The Capital Fund Refactoring

Wherein, we extract the value from a factory and leave it defunct. Just like a real capital fund, we also ignore the plight of the workers in that factory who now struggle to feed their families.

An exciting and representative example

Consider this super useful factory:

public class ThingFactory
{
    public string ThingName { get; }
    public string ThingValue { get; }

    public ThingFactory(string thingName, int thingValue)
    {
        if (thingName == null)
            throw new ArgumentNullException(nameof(thingName));
            
        ThingName = thingName;
        ThingValue = thingValue;
    }
    
    public Thing MakeThing(string elusiveThirdParameter)
    {
        if (elusiveThirdParameter == null)
            throw new ArgumentNullException(nameof(elusiveThirdParameter));
            
        return new Thing(ThingName, ThingValue, elusiveThirdParameter);
    }
}

It looks trivial, but it's honestly not far from a factory I really saw out in the wild recently - you're welcome to imagine a slightly more useful example if it helps though.

The example I found was used only twice and both of those in the same ApiController, both called like this:

//lurking within other code

var thingFac = new ThingFactory(myThingName, myThingVal);
var thing = thingFac.MakeThing(thirdParam);

So it was trivial to apply the Capital Fund refactoring, and move the object creation to a private static method nearby:

private static Thing MakeThing(string thingName, int thingVal, string thirdParam)
{
    if (thingName == null)
        throw new ArgumentNullException(nameof(thingName));
    if (thirdParam == null)
        throw new ArgumentNullException(nameof(thirdParam));
        
    return new Thing(thingName, thingVal, thirdParam);
}

What even is the point of that?

It's easy to dismiss this change as pointless, and in such a trivial example it mostly is - but, for factories that are only used in one place it's a good way to remove an unnecessary and potentially confusing abstraction so that you can find a better one.

To be clear, I am not saying the factory pattern is always a bad idea, but often it is used without considering the value that it adds and can often be the wrong thing to use in a given circumstance - just removing the factory often isn't enough to improve the readability of the code, but by moving the methods it provides closer to where they are used it can be much easier to reason about the code and find a better abstraction. So, it is worth considering these points before mindlessly applying this refactoring:

Guidelines

Good times to apply this:

• When a factory is used very few times
• When a factory can trivially be replaced with a constructor call
• When a factory very clearly returns a concrete type

Bad times to apply this:

• When a factory is used in a bunch of places
• When a factory nicely separates out logic about creating something that would otherwise be boring and repetitive

Is this the end?

The Capital Fund refactoring helps remove an unnecessary indirection from your code, but it's important to note that this refactoring alone doesn't necessarily leave you with better code. The aim of this refactoring is to make things clearer so that you can continue on to develop an abstraction that makes more sense. Unless the use of a factory was completely pointless, in which case you might just want to offer mean kind words of support to whoever added it in the first place, in the hope that you don't have to apply this refactoring too many times.