javasolid-principlesopen-closed-principle

Clean code for removing switch condition(using polymorphism)


As SOLID principles say, it's better to remove switch conditions by converting them to classes and interfaces. I want to do it with this code:

Note: This code is not real code and I just put my idea into it.

MessageModel message = getMessageFromAnAPI();
manageMessage(message);
...
void manageMessage(MessageModel message){        
    switch(message.typeId) {
        case 1: justSave(message); break;
        case 2: notifyAll(message); break;
        case 3: notify(message); break;
    }
}

Now I want to remove switch statement. So I create some classes for it and I try to implement a polymorphism here:

interface Message{
    void manageMessage(MessageModel message);
}
class StorableMessage implements Message{

    @Override
    public void manageMessage(MessageModel message) {
        justSave(message);
    }
}
class PublicMessage implements Message{

    @Override
    public void manageMessage(MessageModel message) {
        notifyAll(message);
    }
}
class PrivateMessage implements Message{

    @Override
    public void manageMessage(MessageModel message) {
        notify(message);
    }
}

and then I call my API to get my MessageModel:

MessageModel message = getMessageFromAnAPI();

Now my problem is here. I have my model and I want manage it using my classes. As SOLID examples, I should do something like this:

PublicMessage message = new Message();
message.manageMessage(message);

But how can I know which type is related to this message to make an instance from it(PublicMessage or StorableMessage or PrivateMessage)?! Should I put switch block here again to do it or what?


Solution

  • You can use a factory in this case to get the instance of Message. The factory would have all instances of Message and returns the appropriate one based on the MessageModel's typeId.

    class MessageFactory {
        private StorableMessage storableMessage;
        private PrivateMessage privateMessage;
        private PublicMessage publicMessage;
        //You can either create the above using new operator or inject it using some Dependency injection framework.
    
        public getMessage(MessageModel message) {
            switch(message.typeId) {
                case 1: return storableMessage; 
                case 2: return publicMessage;
                case 3: return privateMessage
                default: //Handle appropriately
            }
        }
    }
    

    The calling code would look like

    MessageFactory messageFactory; //Injected 
    ...
    MessageModel messageModel = getMessageFromAnAPI();
    
    Message message = messageFactory.getMessage(messageModel);
    message.manageMessage(messageModel);
    

    As you can see, this did not get rid of the switch entirely (and you need not as using switch is not bad in itself). What SOLID tries to say is to keep your code clean by following SRP (Single Responsibility Principle) and OCP (Open-Closed Principle) here. What it means here is that you code shouldn't have the actual processing logic to handle for each typeId in one place.

    With the factory, you have moved the creation logic to a separate place and you have already moved the actual processing logic to respective classes.

    EDIT: Just to reiterate - My answer focuses on the SOLID aspect of the OP. By having separate handler classes (an instance of Message from the OP) you achieve the SRP. If one of the handler classes changes, or when you add a new message typeId (message.typeId) (i.e, add a new Message implementation) you need not modify the original and hence you achieve OCP. (On assumption that each of these does not contain trivial code). These are already done in the OP.

    The real point of my answer here is to use a Factory to get a Message. The idea is to keep the main application code clean and limit the usages of switches, if/else and new operators to instantiation code. (Similar to @Configuration classes/ the classes that instantiate Beans when using Spring or Abstract modules in Guice). The OO principles do not say using switches are bad. It depends on where you use it. Using it in the application code does violate the SOLID principles and that is what I wanted to bring out.

    I also like the idea from daniu@ to use a functional way and the same can even be used in the above factory code (or can even use a simple Map to get rid of the switch).