Are static classes/methods good for pure business logic?

Question

I have a service class that performs some operations. One of the operations is a piece of code long enough to warrant extracting to a new class and unit test it in isolation:

@Service
public class ItemFinalizer {
    private final ItemPublisher itemPublisher;
    private final FollowupScanPublisher followupScanPublisher;

    public void finalizeItem(Item item) {
        itemPublisher.publish(item);
        handleFollowUpScan(item);
    }

    private void handleFollowUpScan(Item item) {
        ScanType followUpScanType = null;
        if (item.getScanType() == ScanType.STANDARD) {
            if (item.getBank().containsFlag(BankFlagEnum.AUTO_STANDARD_TO_ADVANCED)) {
                followUpScanType = ScanType.ADVANCED;
            } else if (item.getBank().containsFlag(BankFlagEnum.AUTO_STANDARD_TO_FULL_BOARDING)) {
                followUpScanType = ScanType.FULL_ONBOARDING;
            }
        } else if (item.getScanType() == ScanType.SIGNUP &&
                item.getBank().containsFlag(BankFlagEnum.SINGUP_BOARDING_COMPLETE_CYCLE)) {
            followUpScanType = ScanType.FULL_BOARDING;
        }

        if (followUpScanType != null && item.isRegistered()) {
            FollowUpScanRequest followUpScanRequest = new FollowUpScanRequest(item.getId(), followUpScanType.getId());
            followupScanPublisher.publish(JsonUtils.toJson(followUpScanRequest));
        }
    }
}

After extracting the first 11 lines of the handleFollowupScan method (pure business logic) to an external class, it looks like this:

private void handleFollowUpScan(Item item) {
    ScanType followUpScanType = FollowupScanExtractor.extractFollowupScan(item);
    if (followUpScanType != null && item.isRegistered()) {
        FollowUpScanRequest followUpScanRequest = new FollowUpScanRequest(item.getId(), followUpScanType.getId());
        followupScanPublisher.publish(JsonUtils.toJson(followUpScanRequest));
    }
}

The question is - should the new method (FollowupScanExtractor::extractFollowupScan) be static?

On one hand, there's no reason for it not to be static, as this is a pure function with a deterministic result (you'll always get the same output for the same input). It will also never be mocked, and can be very easily tested.

On the other hand, this isn't a "utility" class per se, as the code in there is purely business logic, hence it kind of violates the mainstream usage of static classes.

I considered extracting it to a regular (non-static) class or create a spring bean (@Component) for holding that logic (I'm running inside a spring boot container, if that matters).

Answer 1

When making decisions like this in a project where it appears that there are multiple choices we can possibly make, then it almost always makes sense to make the choice that is consistent with the overall architecture and prior design choices.

In this case, while you may not see a specific reason for it not to be static, then I would look to see if making this static would go against the grain of the rest of the design of your application. This looks like a Spring application and you are employing DI/IoC strategies, so the best thing would be to do what you suggest at the end and create a new component bean. Even if you do not have any mockable dependencies at this time, there may come a time later where your business logic changes and you will have something to mock. Keep your options open.

Answer 2

Prior to the addition of method references to Java, making a method static implicitly meant that any use of it was hardcoded. So using static methods inherently limited your options and made your solution less flexible. I think this is perhaps the origin of the idea that only utility methods should be static.

I notice that you refer to method using the :: syntax which suggests that you might be thinking in method reference terms. If you are passing references to this method around, then having it as a member essentially creates an additional parameter. In other words, it's roughly equivalent to declaring the method like this:

static ScanType extractFollowupScan(FollowupScanExtractor extractor, Item item) {...

This isn't just theoretical. You can pass and use member method references in this way. You just need to have a FollowupScanExtractor to pass to the call. This seems to be a lesser known feature of method references in Java.

From that perspective, the idea that you would have to create an object with no state and used only so you can pass it to this method (which doesn't use it) seems odd. However, as maple_shaft argues, if the style of the project is OO-centric, introducing the use of method references could be confusing and go against the grain of the Spring orthodoxy.

Answer 3

The problem with static is you throw away inheritance. You might not be using inheritance today, but its kind of the point of inheritance that you can retroactively use it in the future.

So, no static methods are not good in OOP languages. Don't use them just because you can.

Answer 4

• Depending on state that must be only one copy is a reason to be static.

• Depending on state that must be different each instance is a reason to not be static.

• Not depending on state isn't a reason to be either one.

Static methods can work without state. Objects can work without state.

Needing to not hard code a particular implementation is a reason to not be static. But that only works if you actually follow through and avoid hardcoding with new. The strategy pattern shows a way to do this without containers. Pure DI can handle this with nothing but the language tools. Similarly some languages allow you to inject static methods solving this same problem. Either way, polymorphism, even just as future proofing, is worth a thought. State isn't the only possible consideration.

But that's easy to over engineer. Lets take a hard look at why you're doing this.

can be very easily tested

One thing I've seen people do, and been guilty of myself, is carving up code just to test it more directly. The danger here is you crawl so far into an arbitrary implementation choice that your testing actually starts to make refactoring harder.

So before sweating the static or not issue think seriously about just making extractFollowupScan() a private method. You test it by testing the public method that uses it. If that gives you code coverage you have the testing you need. If it doesn't you have too much code.

If you want to highlight that the method is pure, mention it in the java doc.