How to clean up a business logic class that does too many things without injecting a ton of classes into it?

Question

Let's say we have business logic to perform in an MVC framework after a user submits a form:

class BusinessLogic:
    def sendDataTo3rdPartyApi(self):
        # do stuff
    def validateResponse(self):
        # do stuff 
    def persistData(self):
        # do stuff 
    def calculateFees(self):
        # do stuff 
    def sendDataToAnotherApi(self):
        # do stuff

POST form data is received by the controller and it passes that data to the business logic layer for processing. All of these functions are necessary to process the data. We recognize that this BusinessLogic class is likely doing to many things, so we abstract the various methods into their own classes and inject them via DI:

class BusinessLogic:
    def __init__(self, dataSender, validator, dataPersister, feeCalculator, anotherDataSender):
        # assign objects to private properties within class
    def doLogic(self):
        # utilize the injected classes to perform necessary business logic

The problem I see with this approach is that abstracting the functions into classes that perform one thing still leaves us with a class that does too many things--it's just in the form of classes instead of functions. We can try to move these classes out of this class, but we would just be sending the dependencies up into the controller. As our business logic gets more complex, the more we're going to have to inject since we're creating a class for each particular business requirement.

I feel like no matter what you do, you'll always require a god class of some sort to organize the different parts together for execution. Now I'm unsure if this is necessarily a bad thing, but I'm wondering if there is a better approach to refactoring a business logic class like this.

Does this answer your question? [Using a "Pass-through (God) Service" is bad, right?](https://softwareengineering.stackexchange.com/questions/384363/using-a-pass-through-god-service-is-bad-right) — gnat, Jun 02 '20 at 20:57
see also: [What are the practical ways to implement the SRP?](https://softwareengineering.stackexchange.com/q/158845/31260) — gnat, Jun 02 '20 at 20:59
Yes, I can understand why it's a bad practice. I can remove the pass-through service, however, all I would be doing is moving the business logic into the controller. How can I connect all of these business logic pieces together without the help of a pass-through service or god (controller) function? — Robert Calove, Jun 02 '20 at 22:52

Doc Brown · Answer 1 · 2020-06-04T07:48:04.883

There is nothing wrong per se with a class BusinessLogic which orchestrates the logic of calling methods of a huge list of other objects, as long as one does not mix up different levels of abstraction.

When BusinessLogic only connects the data flow between objects like dataSender, validator, dataPersister, feeCalculator, anotherDataSender, ..., it does not do "too many things" - it actually has only one responsibility: managing the data flow, nothing more, nothing less. If one manages it to keep any real business logic out of BusinessLogic (pun intended), even unit tests for this class are not required any more, an integration test will usually be enough.

^{(Side note: I would rethink if BusinessLogic is the right name for this class.)}

Problems will only occur if BusinessLogic is still having some responsibilities for parts of the business logic by itself, and mixes these up with the orchestration task. That will lead to a system which becomes hard to test, hard to maintain and thus hard to evolve.

For example: this is fine

def doLogic(self,data):
     response = dataSender.send(data)
     validator.validate(response)
     dataPersister.persistData(data)
     feeData = feeCalculator.calculateFees(data)
     anotherDataSender.send(data,feeData)

and this is the road to hell:

def doLogic(self,data):
     response = dataSender.send(data)
     if(response.isValid())
     {
         validator.validate(response)
         for i in range(maxRetries)
             if(persistData(data).ok())
                  break

          feeData = feeCalculator.calculateFees(data)
          anotherDataSender.send(data,feeData)
     }

One can get rid of such constructs by moving the decision logic completely into the injected classes, and model the input and output data in a way it can represent failed steps, empty data sets or "null objects", whatever is most appropriate in the specific context.

So as long BusinessLogic contains pure orchestration logic, and does not mix up different abstraction levels, it is not a god class, even if one has to inject a dozen different objects into it.

This is a good answer, but I feel like it is missing the obvious red flag in the question: the `calculateFees` method. Judging from the name, it looks to be straight-up business logic clearly intermingled with other concerns. — Greg Burghardt, Jun 03 '20 at 11:23
@GregBurghardt: why do you think a `calculateFees` method inside a separate `feeCalculator` class is "intermingled with other concerns"? — Doc Brown, Jun 03 '20 at 11:27
I'm speaking more of the question asker's code than the code you posted. The person who asked the question has a method called `calculateFees` in a class that clearly doing data persistence, validation and communication with outside APIs. — Greg Burghardt, Jun 03 '20 at 11:28
@GregBurghardt: let me cite the OP: *"We recognize that this BusinessLogic class is likely doing to many things, so we abstract the various methods into their own classes"* (and then showing the new BusinessLogic with an injected feeCalculator etc) - to my reading, the OP already made the separation you are suggesting. — Doc Brown, Jun 03 '20 at 11:32
Ugh. It's early. I need coffee. I missed that glaring fact in the question. — Greg Burghardt, Jun 03 '20 at 11:33

score 0 · Answer 2 · answered Jun 03 '20 at 01:39

Presumably not everything that relies on business logic, must have or even need access to all of the functions in this class.

Why not look for a natural cleave point? A good cleave would be one that reduces the amount of users needing both halves, or where the implementation doesn't need the other half of the class to achieve its goal.

Another trick would be to push some of these implementation details down into specialised logic classes, and then relate their instances via an umbrella class.

How to clean up a business logic class that does too many things without injecting a ton of classes into it?

2 Answers2