Replace switch-case with polymorphism

Question

I know there are similar questions already, but looking at them I still have some doubts about how I should design my code. I have a service that allows for User registration / login /update / delete. The thing is that the User is an abstract type, which contains the data typeOfUser based on which the actual registration / update / delete methods should be called, and right now I do that in a switch-case block. I'd like to replace that with some better design.

UserController.java

public class UserController {

    public UserDto register(UserDto user) {
        switch(user.getTypeOfUser()) {
        case DRIVER: return driverService.register(user);
        case CUSTOMER: return customerService.register(user);
        // ...
        }
    } 

    public UserDto update(UserDto user) {
        switch(user.getTypeOfUser) {
        case DRIVER: return driverService.update((DriverDto) user);
        case CUSTOMER: return customerService.update((CustomerDto) user);
        // ...
        }
    }

    public UserDto login(long userId) {
        loginService.login(userId);

        UserBO user = userService.readById(userId);

        switch(user.getTypeOfUser) {
        case DRIVER: return DriverDto.fromBO((DriverBO) user);
        case CUSTOMER: return CustomerDto.fromBO((CustomerBO) user);
        // ...
        }
    }

    // ...
}

I understand that something like Visitor pattern could be used, but would I really need to add the methods of registration / login /update / delete in the Enum itself? I don't really have a clear idea on how to do that, any help is appreciated.

Nocturn

The trick would be to have a single general service, and then just call it once with a user as input, i.e. call service.register(user) once instead of having a switch statement. – Luing

@TimBiegeleisen Yes, but how would you implement the "update(user)"? Because update works with Client or Driver specific fields. So it can't really use the general service, it has to use specific. – Nocturn

I am pointing out one way in which polymorphism can help you. You may need such a switch statement somewhere, though it doesn't have to be here. – Luing

@Nocturn The Visitor pattern solves a very specific problem and almost always an overkill in most cases (if not all). See my answer for a way to address this with simple Polymorhism.. – Saville

Useful context: refactoring.com/catalog/replaceConditionalWithPolymorphism.html – Excess

Oops, since you have a typeCode I think it's this refactoring: refactoring.com/catalog/replaceTypeCodeWithPolymorphism.html – Excess

Answer 1

I'd like to replace that with some better design.

The first step towards replacing the switch statement and take advantage of Polymorphism instead is to ensure that there is a single contract (read method signature) for each of the operations regardless of the user type. The following steps will explain how to achieve this :

Step 1 : Define a common interface for performing all operations

interface UserService {
    public UserDto register(UserDto user);
    public UserDto update(UserDto user);
    public UserDto login(UserDto user)
}

Step 2 : Make UserController take a UserService as a dependency

public class UserController {
     
    private UserService userService;
    
    public UserController(UserService userService) {
        this.userService = userService;
    }
    
    public UserDto register(UserDto user) {
       userService.register(user);
    } 

    public UserDto update(UserDto user) {
        userService.update(user);
        
    }

    public UserDto login(long userId) {
        userService.login(user);
    }
      
}

Step 3 : Create subclasses to handle different types of users that take CustomerDto and CustomerBO as a dependency

class CustomerService implements UserService {
    private CustomerDto userDto;
    private CustomerBO userBO;
    
    public CustomerService(UserDto userDto,UserBO userBo) {
          this.userDto = (CustomerDto)userDto;
          this.userBO= (CustomerBO)userBo;
    }

    //implement register,login and update methods to operate on userDto and userBo
}

Implement the DriverService class in a similar fashion with a dependency on DriverBo and DriverDto objects respectively.

Step 4 : Implement a runtime factory that decides which service to pass to UserController :

public UserControllerFactory {
    public static void createUserController(UserDto user) {
       if(user.getTypeOfUser().equals(CUSTOMER)) { 
           return new UserController(new CustomerService(user));
       } else if(user.getTypeOfUser().equals(DRIVER)) {
           return new UserController(new DriverService(user));
       }
    }
 
}

Step 5 Call the factory to create a user controller

UserDto user = someMethodThatCreatesUserDto(();
UserController controller = UserControllerFactory.createUserController(user);
controller.register();
controller.update();
controller.login();

The advantage of the above approach is that the switch/if-else statements are moved all they way back to a single class i.e the factory.

Saville

Probably the best explanation of polymorphism that I have seen on the internet. – Stace

@Stace Thank you!! That is the best compliment I've got in all my years of contributing to StackOverflow ;) I have some more answers on this topic 1) Polymorphism vs the Strategy Pattern 2) What can Polymorphism do that Inheritance can't? 3) What design pattern does Collections.sort use? – Saville

Just a quick note, do you think it would be better to replace the conditionals in the factory class with an array of keys? Something like return new UserController(services[user.getTypeOfUser()]) where the key/value pair would match the class instance ? – Stace

@Stace Yes that would make sense. I have an answer on this topic as well :). The reason why I chose to show 'if-else' conditions in my question is to highlight the fact that conditionals will be required even when we take advantage of Polymorphism,. The only difference being that the conditions will be in a single place where the objects are wired together. In the real world, an IoC framework such as Spring would be used to wire the objects together. – Saville

Answer 2

You'd want something like that:

public abstract class User {
    abstract void register();
    abstract void update();
    abstract void login();

    // maybe some more common non-abstract methods
}

Any type of User will have a class that extends this abstract class and therefore must implement all its abstract methods, like this:

public class Driver extends User {
    public void register() {
        // do whatever a driver does when register...
    }
    public void update() {
        // do whatever a driver does when update...
    }
    public void login() {
        // do whatever a driver does when login...
    }
}

public class Customer extends User {
    public void register() {
        // do whatever a customer does when register...
    }
    public void update() {
        // do whatever a customer does when update...
    }
    public void login() {
        // do whatever a customer does when login...
    }
}

This way, you're avoiding any switch case code. For instance, you can have an array of Users, each one them will be instantiated using new Driver() or new Customer(). Then, for example, if you're iterating over this array and executing all the Users login() method, each user's login() will be called according to its specific type ==> no switch-case needed, no casting needed!

Treharne

The problem with this is that the "register(UserDto user)" method can only decide which type of user is registering based on the "typeOfUser" field. I thought creating a separate registration endpoint for every kind of user would not be scalable, do you think I'm wrong? – Nocturn

@Nocturn in my implementation you don't need the UserDto user parameter at all and that's exactly the nice thing about it, and about polymorphism. The implementation is type-specific, so the class Driver will have its own way of registering, updating and logging in, same goes for Customer. And that's exactly what makes it scalable, once you have a new type of User you create a new class that extends User, implement the abstract methods and you good to go, you don't have to touch any of the other existing classes. – Treharne

ok, but when I send you a user object json over the network, how would you determine whose .register) method you will call? You have to call the method of a concrete type, and type info is only stored in "typeOfUser" field in a json – Nocturn

@Nocturn JSON is a different thing, it's unrelated to the business logic layer. But if I understand you correctly, according to the typeOfUser field in your JSON you'll instantiate the right user. So if it's typeOfUser: 'driver' you'll go User u1 = new Driver(). Then, when you'll call u1.register(), the right register() will be called according to the runtime type of u1 which is Driver in this case. – Treharne

@Nocturn when you unmarshal the JSON, you have to deal with making the object its correct type. If you're using JAXB this could be useful. #33370290 – Excess

Answer 3

If you really need that TypeOfUser-enum in your UserDTO, then you could extend your enum with a serivce. So you create a TypeOfUserService interface. CustomerSerivce and DriverService will inherit from that service:

    public interface TypeOfUserService {
       public void register(UserDTO user);
       // ...
    }

    public class CustomerService implements TypeOfUserService {
       @Override
       public void register(UserDTO user) {
         // ...
       }
    }

    public class DriverService implements TypeOfUserService {
       @Override
       public void register(UserDTO user) {
         // ...
       }
    }

Then you create your register, update, etc. methods in your TypeOfUser enum:

public enum TypeOfUser {

  DRIVER(new DriverService()), 
  CUSTOMER(new CustomerService());

  private TypeOfUserService typeOfUserService;

  TypeOfUser(TypeOfUserService typeOfUserService) {
    this.typeOfUserService = typeOfUserService;
  }

  public static void register(String typeOfUser, UserDTO user) {
     TypeOfUser.valueOf(typeOfUser).typeOfUserService.register(user);
  } 
  // ...

 }

You could then call the register method via:

    class UserController() {
      public UserDto register(UserDto user) { 
        TypeOfUser.register(user.getTypeOfUser, user);
      }
    }

Underfeed

Answer 4

Very simple example (only for different login logic for DriverDto and CustomerDto) - I've resigned from field typeOfUser (because it is not necessary in my solution) - I'm not sure that this is possible in your solution:

public abstract class UserDto {
    // put some generic data & methods here
}

public class CustomerDto extends UserDto {

    private String customerName;

    public String getCustomerName() {
        return customerName;
    }

    public void setCustomerName(String customerName) {
        this.customerName = customerName;
    }
}

public class DriverDto extends UserDto {

    private String driverName;

    public String getDriverName() {
        return driverName;
    }

    public void setDriverName(String driverName) {
        this.driverName = driverName;
    }
}

public class ThisIsServiceOrDelegateToOtherServices {

    public void login(CustomerDto customer) {
        String name = customer.getCustomerName();
        System.out.println(name);
        // work on name here
    }

    public void login(DriverDto customer) {
        String name = customer.getDriverName();
        System.out.println(name);
        // work on name here
    }

}

Usage:

public static void main(String... args) {
    //demo data
    CustomerDto customer = new CustomerDto();
    customer.setCustomerName("customerName");
    DriverDto driver = new DriverDto();
    driver.setDriverName("driverName");
    // usage
    ThisIsServiceOrDelegateToOtherServices service = new ThisIsServiceOrDelegateToOtherServices();
    service.login(customer);
    service.login(driver);
}

Closet