Factory/Strategy Pattern for objects that require different pieces of data

Question

I'm facing some difficulty with designing a factory and/or strategy pattern for building out EmailTemplates. IMO this seems like the design pattern to go with, but I feel like the path I'm going down isn't very extensible and will be prone to spaghetti-like code in the future. The problem is this: an EmailService takes in a (hydrated) EmailTemplate to send an email. The EmailTemplate has a bunch of key/value pairs that are derived from different objects/data depending on the strategy (receipt, refund, void, etc).

Here is what I'm currently working with:

public class EmailTemplateFactory {

private static Map<TemplateType, BiFunction<EmailTemplate, EmailTemplateContext, EmailTemplate>> templates = new HashMap<>();

static {
    templates.put(TemplateType.RECEIPT, EmailTemplateFactory::buildReceipt);
    templates.put(TemplateType.REFUND, EmailTemplateFactory::buildRefund);
    templates.put(TemplateType.VOID, EmailTemplateFactory::buildVoid);
}

public static EmailTemplate getTemplate(final EmailTemplateContext emailTemplateContext) {
    if (emailTemplateContext == null) {
        throw new IllegalArgumentException("Must provide context to build email template!");
    }
    return templates.get(emailTemplateContext.getTemplateType())
            .apply(getBaseTemplate(TemplateFormat.HANDLEBARS, emailTemplateContext), emailTemplateContext);
}

private static EmailTemplate buildReceipt(final EmailTemplate baseTemplate, final EmailTemplateContext emailTemplateContext) {
    //build the receipt template
    return new EmailTemplate();
}

private static EmailTemplate buildVoid(final EmailTemplate baseTemplate, final EmailTemplateContext emailTemplateContext) {
    // build the void template
    return new EmailTemplate();
}

private static EmailTemplate buildRefund(final EmailTemplate baseTemplate, final EmailTemplateContext emailTemplateContext) {
    // build the refund template 
    return new EmailTemplate();
}

private static EmailTemplate getBaseTemplate(final TemplateFormat templateFormat, final EmailTemplateContext emailTemplateContext) {
  // setting up common data between templates 
}

public enum TemplateType {
    RECEIPT, REFUND, VOID;
}
}

I created a class called EmailTemplateContext which contains the multiple different pieces of data that these different strategies use to build out the template. However, this is what feels troublesome, because its lots of somewhat unrelated fields where only 1 or 2 (of the 5 or 6) are needed to build any given template. Is a context object being passed into the factory the right approach here?

Most of the code inside of the buildReceipt, buildRefund, and buildVoid methods is simply emailTemplate.addVariable(key, value), and a fair amount of them are shared among strategies (why I created the getBaseTemplate method.

Any help would be appreciated to point me in the right direction. Thanks!

Answer 1

Why not use the "classic" approach to the strategy pattern with a base class EmailTemplateStrategy, a pure virtual method buildTemplate, and derivated classes like BuildReceiptTemplateStrategy, BuildVoidTemplateStrategy, BuildRefundTemplateStrategy?

This is technically not much different from what you have now, but it has the following advantages

• A protected member of type EmailTemplateContext can be part of the base class, either passed in through the constructor, or simply created directly inside the base class constructor. So the caller/user of BuildFooTemplateStrategy does not have to pass the same argument with each call.

• EmailTemplateStrategy will be the natural place for some reusable utility methods. These could provide a full API to EmailTemplateContext, so the derivations won't access the context object directly, only through this API. This gives you the opportunity to refactor the EmailTemplateContext in case you think it becomes too unstructured.

• If the different strategies become more complex, the code will be better organized: instead of one huge god class EmailTemplateFactory, you get several classes which contain exactly the code for the specific template, each one maybe having additional private helper methods or maybe additional member variables.

(I was tempted to add here something about the OCP, and easier extensibility, but thinking twice I guess your current approach allows this, too, since it is probably not really mandatory to have new template building functions inside the factory class.)

Of course, you will still need a factory class for providing a map Map<TemplateType,EmailTemplateStrategy>, but this factory then will only hold the code for the construction of this mapping, but not the whole code base for the email templates as well.

Beware, what I suggested aims for a growing code base with several hundred lines of code (or more), in expectation of new requirements. If your program only provides 3 or 4 different email templates, and there are no new requirements to be expected in the near future, then I would not bother refactoring it.