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?
Best Answer
What's the difference between
new CustomerReceiptCreator().createReceipt()
andCustomerReceiptCreator.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 ofnew
ing it. Let's consider an example:Let's compare this a static method version:
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:Then we are done, because the calling code will be injected with a
CustomerReceiptCreator
which will automatically get injected aFeatureFlags
.What if we were using a static method?
But wait! the calling code also needs to be updated:
Of course, this still leaves the question of where
processOrder
gets itsFeatureFlags
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.