8

For a project, we have a Controller/Service/DAO architecture. We implement calls to different providers' APIs so we ended up with some boilerplate code like this in every controller class:

enum {
    PARTNER_A, PARTNER_B, PARTNER_C
}

public class MyController {
    @Resource PartnerASearchService partnerASearchService;
    @Resource PartnerBSearchService partnerBSearchService;
    @Resource PartnerCSearchService partnerCSearchService;

    public search(InputForm form) {
        switch(form.getPartnerName()) {
           case PARTNER_A: partnerASearchService.search();
                            break;
           case PARTNER_B: partnerBSearchService.search();
                            break;
           case PARTNER_C: partnerCSearchService.search();
                            break;
        }
    }

    public otherMethod(InputForm form) {
        switch(form.getProvider()) {
           case PARTNER_A: partnerAOtherService.otherMethod();
                           break;
           case PARTNER_B: partnerBOtherService.otherMethod();
                           break;
           case PARTNER_C: partnerCOtherService.otherMethod();
                           break;
        }
    }
}

Which design pattern can I use to get rid of this switch in every controller? I would prefer the code to be something like the below:

public class MyController {
    @Resource ServiceStrategy serviceStrategy;

    public search(InputForm form){
        serviceStrategy.search(form.getPartnerName())
        // or
        serviceStrategy.invoke(SEARCH, form.getPartnerName())
    }

    public otherMethod(InputForm form){
        serviceStrategy.other(form.getPartnerName())
        // or
        serviceStrategy.invoke(OTHER, form.getPartnerName())
    }
}

letting the serviceStrategy decide which service implementation to be called, and thus having the partner's switch in a single place.

I've used the term "strategy" because I've been told this design pattern could make it, but I'm not sure of the best way to use it or if there is a better approach to solve this problem.

EDIT: I've updated the question as the term provider is misleading. What I have in the input form is the name of the partner for which we do the request. I want a pattern that decides which is the correct implementation (which one of the several services) to use based on the partner's name in the form

luso
  • 2,812
  • 6
  • 35
  • 50

5 Answers5

2

Generally, the form shouldn't need any knowledge of what "provider" is going to handle it. Instead, the providers should be able to explain which kinds of inputs they can handle.

I recommend using a form of Chain of Responsibility (incorporating the refactoring Replace Conditional with Polymorphism) that looks something like this (Groovy for simplicity):

interface ProviderService {
    boolean accepts(InputForm form)
    void invoke(String foo, InputForm form)
    void other(InputForm form)
}

Each implementation of ProviderService implements accepts to indicate whether it can handle a particular form, and your controller stores a List<ProviderService> services instead of individual references. Then, when you need to process a form, you can use:

ProviderService service = services.find { it.accepts(form) }
// error handling if no service found
service.other(form)

See the Spring conversion service for a comprehensive example of this pattern that's used in a major framework.

chrylis -cautiouslyoptimistic-
  • 75,269
  • 21
  • 115
  • 152
0

Indeed, you can use the Strategy pattern here. It will look like something like this.

Here, you have to get the designated ServiceProvider from InputForm.

You can have StrategyMaker class something like this.

Public class StrategyMaker{

    public SeriveProvider getProviderStrategy(InputForm inputForm){
        return inputForm.getProvider();
    }
}

And inside controllers you can do something like this.

public class MyController{
    StrategyMaker strategyMaker;

    public search(InputForm form){
        strategyMaker.getProviderStategy(form).search();
    }
}

This will be an ideal solution, if you know list of all the provider strategies forehand. Strategy pattern unable to keep Open-Close-Princple when the list is continuously growing.

And one other thing is when you refer a pattern, always try to get the big picture. Don't rigidly look into the implementation example any source provides. Always remember that it's an implementation, not the implementation.

Supun Wijerathne
  • 11,964
  • 10
  • 61
  • 87
  • 1
    Can the down voter please list down flaws in this design, so that other users get better picture? – JavaHopper Aug 08 '16 at 15:24
  • @JavaHopper I also believe that a down vote should have a description, at least, I can know that what's wrong according to him, so that we can argue on any point or can solve any misunderstandings if any. I can't understand what I did wrong here. :( – Supun Wijerathne Aug 08 '16 at 15:31
  • 1
    Inputform is about the data in the request, it should not have a reference to a ServiceProvider object. And why do you need to put a StrategyMaker class in between, instead of directly calling inputForm.getProvider? If your StrategyMaker implementation is just a "stub" example, please mention that in the answer. – jhyot Aug 08 '16 at 15:52
  • @jhyot 1) If you read carefully, inputForm has a provider with it, according to the question. It's not my creation. And I don't see much problem in that approach either. 2) About StratergyProvider, yes it can be called off directly, but having a StrategyMaker make that much clear design. If we remove all that types of abstraction from our codes, there will not be Design Patterns at all right? 3) Agree with you, that's why I have mentioned 'something' like this, so that I am outlining the solution. :) – Supun Wijerathne Aug 09 '16 at 02:51
  • maybe the term provider is misleading in the question. Provider in my context is just some partner which proposes an API. We user different partner's APIs so I what I want is a strategy to know which partner's API to use. In the input form we do have just the name of the partner – luso Aug 13 '16 at 15:08
  • @Iuso so, Have I interpreted that in wrong way? As I understand I am doing exactly what you propose. isn't it?? :) – Supun Wijerathne Aug 15 '16 at 03:04
0

First remove the duplication of the provider lookup code by extracting it to an own method.

public class MyController {

    @Resource
    ProviderService searchServiceProvider1;
    @Resource
    ProviderService searchServiceProvider2;
    @Resource
    ProviderService searchServiceProvider3;

    public void search(InputForm form) {
        String provider = form.getProvider();
        ProviderService providerService = lookupServiceProvider(provider);
        providerService.search();
    }

    public void other(InputForm form) {
        String provider = form.getProvider();
        ProviderService providerService = lookupServiceProvider(provider);
        providerService.other();
    }

    private ProviderService lookupServiceProvider(String provider) {
        ProviderService targetService;
        switch (provider) {
        case PROVIDER_1:
            targetService = searchServiceProvider1;
            break;
        case PROVIDER_2:
            targetService = searchServiceProvider2;
            break;
        case PROVIDER_3:
            targetService = searchServiceProvider3;
            break;
        default:
            throw new IllegalStateException("No Such Service Provider");
        }
        return targetService;
    }

}

At least you can improve the lookupServiceProvider method and use a map to avoid the switch.

private Map<String, ProviderService> providerLookupTable;

private Map<String, ProviderService> getProviderLookupTable(){
    if(providerLookupTable == null){
        providerLookupTable = new HashMap<String, ProviderService>();
        providerLookupTable.put(PROVIDER_1, searchServiceProvider1);
        providerLookupTable.put(PROVIDER_2, searchServiceProvider2);
        providerLookupTable.put(PROVIDER_3, searchServiceProvider3);

    }
    return providerLookupTable;
}

private ProviderService lookupServiceProvider(String provider) {
    Map<String, ProviderService> providerLookupTable = getProviderLookupTable();
    ProviderService targetService = providerLookupTable.get(provider);
    if(targetService == null){
        throw new IllegalStateException("No Such Service Provider");
    }

    return targetService;
}

Finally you will recognize that you can introduce a ProviderServiceLocator, move the lookup logic to this class and let MyController use the ProvicerServiceLocator.

A detailed explanation and example code about service provider interfaces and service provider lookup with standard java can be found in my blog A plug-in architecture implemented with java.

René Link
  • 48,224
  • 13
  • 108
  • 140
  • Actually that was a typo. The 2nd method in the controller doesn't call the searchServices but otherServiceProvideX. I've update my question – luso Aug 06 '16 at 11:04
0

One possible solution when the Partners are not frequently changing:

class ServiceFactory {

  @Resource PartnerService partnerASearchService;
  @Resource PartnerService partnerBSearchService;
  @Resource PartnerService partnerCSearchService;

  public static PartnerService getService(String partnerName){
    switch(partnerName) {
       case PARTNER_A: return partnerASearchService;
       case PARTNER_B: return partnerBSearchService;
       case PARTNER_C: return partnerCSearchService;
  }
}



public class MyController {
    @Resource ServiceFactory serviceFactory;

    public search(InputForm form) {
        serviceFactory.getService(form.getProvider()).search()
    }

    public otherMethod(InputForm form) {
        serviceFactory.getService(form.getProvider()).otherMethod()
    }
}
Kapil Malhotra
  • 135
  • 2
  • 10
0

Mixing ideas from different answers I came up to

ServiceProvider.java A superclass for all the service providers. Contains a map of the different services for each partner

public abstract class ServiceProvider implements IServiceProvider {
  private final Map<ServiceType, IService> serviceMap;

  protected ServiceProvider() {
    this.serviceMap = new HashMap<>(0);
  }

  protected void addService(ServiceType serviceType, IService service) {
    serviceMap.put(serviceType, service);
  }

  public IService getService(ServiceType servicetype, PartnerType partnerType) throws ServiceNotImplementedException {
    try {
      return this.serviceMap.get(serviceType);
    } catch (Exception e) {
      throw new ServiceNotImplementedException("Not implemented");
    }
  }
}

ServiceProviderPartnerA.java there is a service provider for each partner, which are injected with the actual service classes for the different methods.

@Service("serviceProviderPartnerA")
public class ServiceProviderPartnerA extends ServiceProvider {

  @Resource(name = "partnerASearchService")
  private ISearchService partnerASearchService;

  @Resource(name = "partnerABookingService")
  private IBookingService partnerABookingService;

  @PostConstruct
  public void init() {
    super.addService(ServiceType.SEARCH, partnerASearchService);
    super.addService(ServiceType.BOOKING, partnerABookingService);
  }
}

ServiceStrategy.java Injected with the different partners' service providers, it implements the only switch needed in the code and returns the correct service for the correct partner to be used in the controller

@Service("serviceStrategy")
public class ServiceStrategy implements IServiceStrategy {

  @Resource(name = "serviceProviderPartnerA")
  IServiceProvider serviceProviderPartnerA;

  @Resource(name = "serviceProviderPartnerB")
  IServiceProvider serviceProviderPartnerB;

  @Resource(name = "serviceProviderPartnerC")
  IServiceProvider serviceProviderPartnerC;

  public IService getService(ServiceType serviceType, PartnerType partnerType) throws PartnerNotImplementedException {
    switch (partnerType) {
      case PARTNER_A:
        return serviceProviderPartnerA.getService(serviceType, partnerType);
      case PARTNER_B:
        return serviceProviderPartnerB.getService(serviceType, partnerType);
      case PARTNER_C:
        return serviceProviderPartnerC.getService(serviceType, partnerType);
      default:
        throw new PartnerNotImplementedException();
    }
  }
}

SearchController.java finally, in my controllers, I just need to inject the serviceStrategy class and use it to recover the correct service.

@Resource(name = "serviceStrategy")
IServiceStrategy serviceStrategy;

@RequestMapping(value = "/search", method = RequestMethod.GET, produces = "text/html")
@ResponseBody
public String search(@RequestParam(value = "partner", required = true) String partnerType, String... params) {
  ISearchService service = (ISearchService) serviceStrategy.getService(ServiceType.SEARCH, partnerType);
  return service.search(params);
}

So, switch off! Hope this helps someone

luso
  • 2,812
  • 6
  • 35
  • 50
  • First of all I have a pretty straight forward question, Why you need param 'partnerType' for method 'getService' in 'ServiceProvider' class, even that was not used?? And that has been propagated to the higher levels too. – Supun Wijerathne Aug 15 '16 at 03:24
  • Yes, sorry, this is a typo. I'm still fine tuning this... Actually in "getService", when I throw the exception, I add the partnerType so I can know for which partner the service is not implemented... otherwise you can just not pass it, like in this example – luso Aug 15 '16 at 10:49
  • otherwise this implementation seems to work quite well – luso Aug 15 '16 at 10:49