1

I have a Trigger Manager scenario where I delegate the triggers (in other-words subscribe triggers) to different handlers.

For now I have three handler types, I use a switch-case with enum (enum here is the handler type) to redirect to correct handler.

But my code seems not extensible, its not generic and it doesn't follow SOLID principle. Imagine if I need to have more handler

I will be eventually coming and editing my switch case code and I will have more cases where it affects the cyclomatic complexity of my code

Below is my exact code snippet

private static TriggerContext getTriggerContext(TriggerHandlerType triggerHandlerType) throws TriggerHandlerException {
    switch (triggerHandlerType) {
        case DASHBOARD_HANDLER:
            triggerContext = new TriggerContext(new DashboardTriggerHandler());
            return triggerContext;
        case COMPONENT_HANDLER:
            triggerContext = new TriggerContext(new ComponentTriggerHandler());
            return triggerContext;
        case WIDGET_HANDLER:
            triggerContext = new TriggerContext(new WidgetTriggerHandler());
            return triggerContext;
        default:
            LOGGER.error(MIS_CONFIGURED_REQUEST_IS_PROVIDED);
            throw new TriggerHandlerException(TRIGGER_HANDLER_TYPE_GIVEN_IS_NOT_CONFIGURED_IN_THE_LIST_OF_TRIGGER_HANDLERS);

    }

}

Can someone help me to enhance this code in-which I can make it more generic and avoid cyclomatic complexity and follow SOLID Principle along with some design pattern.

Keshan Nageswaran
  • 8,060
  • 3
  • 28
  • 45
  • 3
    This is a question for Stack Exchange Code Review – A Honey Bustard Aug 19 '16 at 17:40
  • Look into a Strategy Pattern. https://en.wikipedia.org/wiki/Strategy_pattern – Taylor Aug 19 '16 at 17:40
  • 2
    You could add a `createTrigger()` factory method on the Enum instances, and just call it like `triggerHandlerType.createTrigger()`. Or one step even more formal: you could create a `TriggerFactory` interface, that you would implement with the Enum, and use that type whenever possible. – ppeterka Aug 19 '16 at 17:41
  • 1
    This is *too broad*. There are *a lot* of ways to make this code more extendable. Maybe look into a good book like "Design patterns" from the Gang of Four? – Polygnome Aug 19 '16 at 17:41
  • I think you should have one TriggerManager for each type of data it's to handle. Make subclasses to handle each specific data type. (Then look at the Trampoline pattern for the opposite idea: https://en.wikipedia.org/wiki/Trampoline_%28computing%29 ) – markspace Aug 19 '16 at 17:45

3 Answers3

1

I think you mean "make code more dynamic", and your problem comes from using objects as primitives.

Rather than switching on the enum object, your enum objects should contain the type to be instantiated:

enum TriggerHandlerType {
    DASHBOARD {
        @Override
        TriggerHandler create() {
            return new DashboardTriggerHandler();
        }
    },
    COMPONENT_HANDLER {
        //...
    };

    abstract TriggerHandler create();
}

getTriggerContext can then call create() to instantiate the handler:

private static TriggerContext getTriggerContext(TriggerHandlerType triggerHandlerType) throws TriggerHandlerException {
    return new TriggerContext(triggerHandlerType.create());
}
Vince
  • 14,470
  • 7
  • 39
  • 84
0

I am not sure about the overall design structure, but the switch can be replaced with a newHandler() method on the enum.

private static TriggerContext getTriggerContext(TriggerHandlerType triggerHandlerType)
    throws TriggerHandlerException
{
    return new TriggerContext(triggerHandlerType.newHandler());
}

In the enum you would implement the method for each type enum as

enum TriggerHandlerType {
    DASHBOARD_HANDLER
    {
        Handler newHandler() { return new DashboardHandler(); }
    },
    ...;
    abstract Handler newHandler();
}
Community
  • 1
  • 1
eckes
  • 10,103
  • 1
  • 59
  • 71
0

You could use a map of configurations for that:

// All your triggers classes should implement this interface
interface TriggerHandler {}

// For example:
public static class DashboardTriggerHandler implements TriggerHandler {
}

// Create your configuration
static Map<TriggerHandlerType, Class> contexts;
static {
    contexts = new HashMap<>();
    contexts.put(TriggerHandlerType.DASHBOARD_HANDLER, DashboardTriggerHandler.class);
    contexts.put(TriggerHandlerType.COMPONENT_HANDLER, ComponentTriggerHandler.class);
    contexts.put(TriggerHandlerType.WIDGET_HANDLER, WidgetTriggerHandler.class);
}

// Return your instance through reflection
public static TriggerContext getTriggerContext(TriggerHandlerType triggerHandlerType) throws TriggerHandlerException, IllegalAccessException, InstantiationException {

    Class className = contexts.get(triggerHandlerType);

    if (className == null) {
        throw new TriggerHandlerException();
    }

    return new TriggerContext((TriggerHandler)className.newInstance());
}
Alexey Soshin
  • 16,718
  • 2
  • 31
  • 40