Is this good design for custom email template?

I read effective Java by Joshua Bloch. And it said use enums over int constant. So I was thinking instead of using hard coded strings I could also use enums. The forgotpassword.txt and confirmation.txt are plain text resource files.

When the class is loaded, I get the contents of the text file and store it inside of a map, since enums are only loaded once.

When I want to create an email template I pass key value map to the EmailTemplate.[TemplateName].create () method and it produces an email text. Anytime I want to create a new template I just have to go to one place and add it.

The only down-side I could think of is if you have hundreds of different email templates. Do you think this is a good design?

```
public enum EmailTemplate
{
FORGOT_PASSWORD("forgotpassword.txt"),
CONFIRM_EMAIL("confirmation.txt");

private final String mFile;

public String getFile()
{
return mFile;
}

EmailTemplate(String file)
{
mFile = file;
}

public String create(Map valuesMap)
{
//lazy load file content into template
if (!lookup.containsKey(this))
{
lookup.put(this, FileUtil.getFileContents((this.getFile())));
}
StrSubstitutor sub = new StrSubstitutor(valuesMap);
return sub.replace(lookup.get(this));
}

private static final Map lookup = new HashMap();
}

public class EmailGenerator
{
public static void sendConfirmationEmail(String emailAddress,
String confirmCode)
{
Map valuesMap = new HashMap();
valuesMap.put("link", confirmCode);

String text = EmailTemplate.CONFIRM_EMAIL.create(valuesMap);
System.out.println(text);
}

public static void sendContactUs(String subject, String email, String message, String name)
{
Map valuesMap = new HashMap();
valuesMap.put("name", name);
valuesMap.put("text", message);
valuesMap.put("email", email);

String text = E

-
Don't use plain words as replacement keys since you might accidentally erase a real "name". For example, in your template, use "Hello {{name}}," and you add "{{" and "}}" to the word you want to replace before calling replace. It's easier to do that than to read each template carefully to make sure the keywords do not appear anywhere where they should not be changed.

-
Hide EmailTemplate inside EmailGenerator and make it private. EmailTemplate is a "dirty" class that should not be exposed publicly: the method create(Map valuesMap) must not be accessible since you need exactly the right arguments in the map.

-
You don't even really need EmailTemplate. If you don't want to reread the files all the time, you can use a "lazy" map, Map textFilesCache (file name -> file text). When you fetch a file, you define a method that checks if textFilesCache(filename) is null and, if it is, it loads the file and adds it to textFilesCache. It's even more efficient than the enum since the template files you never use will never be read. An enum seems overkill here and it make the code less readable.

(If you want to have some fun, you can even take a look at WeakHashMap. It's overkill here since you will only have a few files, but if you had very many and very large files, the JVM can drop some values of a WeakHashMap to free memory.)

StackExchange Code Review Q#56034, answer score: 5