HiveBrain v1.2.0
Get Started
← Back to all entries
snippetcsharpMinor

Using factory pattern to create email templates

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
emailcreatefactorytemplatesusingpattern

Problem

Firstly I try to explain my model.

I have 3 type of notifications which can have N types of email templates related to a specific status.

NotifySupervisor(Vacation vacation);
NotifyAdministration(Vacation vacation);
NotifyApplicant(Vacation vacation);


I've created a type of Factory. I don't consider this a Factory Pattern because I think it's not and I'm trying to find the propoer pattern.

Here is the "Factory":

public class VacationNotificationTemplateFactory : IVacationNotificationTemplateFactory
{
    private readonly IUserService _userService;

    public VacationNotificationTemplateFactory(IUserService userService)
    {
        _userService = userService;
    }

    public VacationNotificationSupervisorTemplate CreateSupervisorTemplate(Vacation vacation)
    {
        return new VacationNotificationSupervisorTemplate(vacation);
    }

    public VacationNotificationAdministrationTemplate CreateAdministrationTemplate(Vacation vacation)
    {
        return new VacationNotificationAdministrationTemplate(_userService, vacation);
    }

    public VacationNotificationApplicantTemplate CreateApplicantTemplate(Vacation vacation)
    {
        return new VacationNotificationApplicantTemplate(_userService, vacation);
    }
}


Here is how I create my templates based on the status:

```
public class VacationNotificationSupervisorTemplate : VacationNotificationTemplate
{
public VacationNotificationSupervisorTemplate(Vacation vacation)
{
switch (vacation.Status)
{
case VacationStatus.Cancelling:

Subject = CancellingTemplateSubject;
Body = "Für Mitarbeiter {0} liegt ein Urlaubsstornierung für den Zeitraum {1} bis {2} vor.\n\n Bitte geben Sie den Antrag im Timesheet frei oder lehnen Sie diesen ab. \n\n";
break;

case VacationStatus.Open:
case VacationStatus.Approved:

Subject = RequestTemplateSubject;
Body = "Für M

Solution

There are two things "wrong" with your Abstract Factory pattern. Neither of them are terrible, but it does mean that you've not implemented the pattern, but some other type of factory that I don't have a name for.

  • The factory should return an instance of the base class or interface. Yours returns a concrete instance of the particular type.



  • The factory itself should adhere to an interface and be broken into one factory class for each concrete type.



For example:

public interface IVacationNotificationTemplateFactory
{
    IVacationNotificationTemplate Create();
}

public class VacationNotificationSupervisorTemplateFactory : IVacationNotificationTemplateFactory
{
    IVacationNotificationTemplate Create()
    {
        return new VacationNotificationSupervisorTemplate(_userService);
    }
}

public class VacationNotificationApplicantTemplateFactory : IVacationNotificationTemplateFactory
{
    IVacationNotificationTemplate Create()
    {
        return new VacationNotificationApplicantTemplate();
    }
 }


There's lots of good reading and examples on Wikipedia.

The other thing I notice is all of the strings embedded in the code. I recently built something similar and had a tough time creating tests that weren't brittle. What I ended up doing was storing the strings in html files, keeping the string formatting stuff in there as well. Then I'd load the file at runtime and "bind" it via string formatting. It makes it feel and act much more like a view than some output that needs to be tested.

Speaking of, be sure to HTML encode any string variables that you're injecting into the templates. Otherwise, you'll end up with invalid HTML in the email body. There's a utility method in the System.Web.UI namespace for it. (I'm just assuming you're using HTML mail, my apologies if you're not.)

I'd consider using a more formal, and strongly typed method of building your XML though. There are several libraries to do this in the .Net framework, so I'll leave that choice to you, but my personal favorite way is with Linq-to-XML.

Code Snippets

public interface IVacationNotificationTemplateFactory
{
    IVacationNotificationTemplate Create();
}

public class VacationNotificationSupervisorTemplateFactory : IVacationNotificationTemplateFactory
{
    IVacationNotificationTemplate Create()
    {
        return new VacationNotificationSupervisorTemplate(_userService);
    }
}

public class VacationNotificationApplicantTemplateFactory : IVacationNotificationTemplateFactory
{
    IVacationNotificationTemplate Create()
    {
        return new VacationNotificationApplicantTemplate();
    }
 }

Context

StackExchange Code Review Q#113300, answer score: 7

Revisions (0)

No revisions yet.