3

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!

  • Template engines are typically usage-agnostic. They accept a (potentially pre-processed) template file and a map of template variable names to corresponding, and return the expanded text. Adding a new template with specific variables should not require any changes to the template engine. – Hans-Martin Mosner Jan 28 at 8:59
3

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.

| improve this answer | |
  • I got a fairly decent implementation of this worked out, and it helps clean up the overall architecture of this, but I still don't know if it's what I am after. Almost all of the code inside of the strategies are simply addVariable("paymentAmount", "$59.99"), etc etc. So it doesn't really feel like a "strategy", just a bunch of different values being loaded into a HashMap that backs the EmailTemplate class. A lot of code (setting of key/values) is duplicated among strategies. I'm ultimately looking for a way to consolidate and make it a little bit more generic, but also be extensible later – ungabunga Jan 28 at 19:16
  • In addition, all strategies here return the same concrete object, EmailTemplate. @doc-brown There is no abstraction or interface here, its just an EmailTemplate with its backing HashMap containing different sets of key/value pairs. These EmailTemplates being returned are simply "hydrated" which are then passed off to the template engine to be rendered (out of my control) – ungabunga Jan 28 at 19:23
  • 1
    @ungabunga "There is no abstraction or interface here" - sure there is; EmailTemplate build(EmailTemplate, EmailTemplateContext) <== is your abstract strategy "interface". The abstraction is not the template itself, but the algorithm that builds it (the strategy); pull those static methods into proper objects, refine the signature as you go along, and put stuff that's specific to a concrete strategy into a corresponding object/type. – Filip Milovanović Jan 28 at 21:09
  • @ungabunga: this is pretty hard to answer without knowing more about the gory details. Maybe you are looking for a more declarative approach to define your email templates? Then you could "invent" some simple, comprehensice "declaration language" and parse this instead of repeating addVariable over and over again?? – Doc Brown Jan 28 at 21:27

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.