CODEWITHSUNDEEP
polymorphismseparation-of-concerns
Rodrigo Ruiz
Rodrigo Ruiz

Reputation: 4355

Why would I need separation of concerns in this case?

void executeRequests() {

    ...

    for (Request request in requests) {
        if (request is Type1Request) {
            ...
        } else if (request is Type2Request) {
            ...
        } else if (...) {

        } ...
    }

}

So basically I have a method that does some common process for a list of requests and then iterate through this list and based on the request type (its specific class) it does some specific processing.

One might argue that I should separate each if else block into a separate classes that has an execute method and then just call the execute inside the for loop because this breaks the Separation of concerns pattern. And I agree! I just don't really know why.

In this case, lets assume that I will never reuse those classes or private functions (or however I decide to refactor). Some people argue that this way is easier to read because you don't have to jump to different files or function to understand everything.

So my question is, why is this piece of code bad?

Thank you.

Upvotes: 1

Views: 96

Answers (2)

Earl Grey
Earl Grey

Reputation: 7466

What action is associated with a particular request is of no concern to the object that executes a particular request because of some external trigger. The concern (handling the consequence) belongs to the request. You thus separate it from the executor because it is there incorrectly. Something internal (an action of certain type) is "outsourced" to other class and the class is dominating that decision, which is a liability and coupling.

It's not a question of why you should optionally do this. It's opposite..it is how could it be any other way. If we do proper object oriented programming.

Upvotes: 0

Etienne Maheu
Etienne Maheu

Reputation: 3255

Simply put, because adding a Type3Request object in your code forces you to edit existing code. Every time you change existing code, you should test all the related modules in your application to ensure that no regression bugs appear in your code. In javascript or other loosely-typed languages, it is very easy to overwrite global data from an if branch. In most strongly-typed languages you can generally avoid this issue, but there is still a possibility of overwriting local data. This means that every time you change this executeRequests method, you have to run all possible requests in your application to ensure that you have not introduced a new issue. I want to make clear that I said all requests, not all requests types. This is potentially thousands of tests to run even if you have only a few request types.

On the other side, keeping this code inside different classes as a polymorphic method ensures that it cannot changes data from the executeRequests method as well as its owner class. This enables you to test only calls related to the type that you are changing instead with the same confidence level. In other words, it reduces the amount of bugs in your code.

By using polymorphism, all the "specific code" that needs to be executed for this request will not only be encapsulated into the Type3Request object, but will actually come with this object. There is no other code file to change in order to add a new request type.

This means that you can provide new request types dynamically as plugins, simply by adding a dll in a folder and without recompiling the existing code. If you do no want to go that far, it still gives you the ability to better separate your workload between all team members by avoiding merges to that executeRequests method.

It also makes it simpler to test code by keeping tightly coupled business logic together. Instead of having to get your request to go through the entire execution process just to test this type specific code, you can just call the method directly from the request object and assert the results.

All of this ensures that if an architecture change happens in your project, you know that there is only one place to look for request related code: the related request type class. If you let this code leak into other classes, you might miss some instances of this code when estimating the complexity of changes or even worse, leave it behind after a large refactor which might trigger bugs.

Upvotes: 1

Related Questions