Are we abusing static methods?

Question

A couple of months ago I started working in a new project, and when going through the code it stroke me the amount of static methods used. Not only utility methods as collectionToCsvString(Collection<E> elements), but also plenty of business logic is kept in them.

When I asked the guy responsible for the rationale behind this, he said it was a way of escaping from Spring's tyranny. It goes something around this thinking process: to implement a customer receipt creation method, we could have a service

@Service
public class CustomerReceiptCreationService {

    public CustomerReceipt createReceipt(Object... args) {
        CustomerReceipt receipt = new CustomerReceipt();
        // creation logic
        return receipt;
    }
}

Now, the guy said that he dislikes having classes unnecessarily managed by Spring, basically because it imposes the restriction that client classes must be Spring beans themselves. We end up having everything managed by Spring, which pretty much forces us to work with stateless objects in a procedural way. More or less what is stated here https://www.javacodegeeks.com/2011/02/domain-driven-design-spring-aspectj.html

So instead of the above code, he has

public class CustomerReceiptCreator {

    public static CustomerReceipt createReceipt(Object... args) {
        CustomerReceipt receipt = new CustomerReceipt();
        // creation logic
        return receipt;
    }
}

I could argue to the point of avoiding Spring managing our classes when possible, but what I don't see is the benefit of having everything static. These static methods are also stateless, so also not very OO. I would feel more comfortable with something as

new CustomerReceiptCreator().createReceipt()

He claims that static methods have some extra benefits. Namely:

• Easier to read. Import the static method and we only need to care about the action, no what class is doing it.
• Is obviously a method free of DB calls, so performance-wise cheap; and it is a good thing to make it clear, so that the potential client does need to go into the code and check for that.
• Easier to write tests.

But I just feel there is something not completely right with this, so I would like to hear some more seasoned developers' thoughts on this.

So my question is, what are the potential pitfalls of this way of programming?

Answer 1

What's the difference between new CustomerReceiptCreator().createReceipt() and CustomerReceiptCreator.createReceipt()? Pretty much none. The only significant difference is that the first case has considerably more awkward syntax. If you follow the first in the belief that somehow avoiding static methods makes your code better OO, you are gravely mistaken. Creating a object just to call a single method on it is a static method by obtuse syntax.

Where things do get different is when you inject CustomerReceiptCreator instead of newing it. Let's consider an example:

class OrderProcessor {
    @Inject CustomerReceiptCreator customerReceiptCreator;

    void processOrder(Order order) {
        ...
        CustomerReceipt receipt = customerReceiptCreator.createReceipt(order);
        ...
    }
}

Let's compare this a static method version:

void processOrder(Order order) {
    ...
    CustomerReceipt receipt = CustomerReceiptCreator.createReceipt(order);
    ...
}

The advantage of the static version is that I can readily tell you how it interacts with the rest of the system. It doesn't. If I haven't used global variables, I know that that the rest of the system hasn't somehow been changed. I know that no other part of the system might be affecting the receipt here If I've used immutable objects, I know that order hasn't changed, and the createReceipt is a pure function. In that case, I can freely move/remove/etc this call without worrying about random unpredictable effects elsewhere.

I cannot make the same guarantees if I've injected the CustomerReceiptCreator. It might have internal state that is changed by the call, I might be affected by or change other state. There might be unpredictable relationships between statements in my function such that changing the order will introduce surprising bugs.

On the other hand, what happens if CustomerReceiptCreator suddenly needs a new dependency? Let's say it needs to check a feature flag. If we were injecting, we could do something like:

public class CustomerReceiptCreator {
    @Injected FeatureFlags featureFlags;

    public CustomerReceipt createReceipt(Order order) {
        CustomerReceipt receipt = new CustomerReceipt();
        // creation logic
        if (featureFlags.isFlagSet(Flags::FOOBAR)) {
           ...
        }
        return receipt;
    }
}

Then we are done, because the calling code will be injected with a CustomerReceiptCreator which will automatically get injected a FeatureFlags.

What if we were using a static method?

public class CustomerReceiptCreator {
    public static CustomerReceipt createReceipt(Order order, FeatureFlags featureFlags) {
        CustomerReceipt receipt = new CustomerReceipt();
        // creation logic
        if (featureFlags.isFlagSet(Flags::FOOBAR)) {
           ...
        }
        return receipt;
    }
}

But wait! the calling code also needs to be updated:

void processOrder(Order order) {
    ...
    CustomerReceipt receipt = CustomerReceiptCreator.createReceipt(order, featureFlags);
    ...
}

Of course, this still leaves the question of where processOrder gets its FeatureFlags from. If we're lucky, the trail ends here, if not, the necessity of passing through FeatureFlags gets pushed further up the stack.

There is a trade-off here. Static methods requiring explicitly passing around the dependencies which results in more work. Injected method reduce the work, but make the dependencies implicit and thus hidden making code harder to reason about.