Interface or switch statement, finding the right pattern

Question

This question has probably been posted before, but I couldn't find it.

I've been writing this sort of thing for so long, I sit down to write something new and just start typing this as if it were my own pattern. A project came up recently and I found myself looking at my own code and started thinking how smelly it looks.

 BackgroundInfoIfYouCare

In this particular library I need to send out emails to users. So far there are 13 canned emails.

Each email has it's own template (I'm using a Razor parser, so the templates are written in cshtml). Each email template has a Name Key of string. Each email has it's own EF4 query to return a model based on a "membership" entity and all related data.

I have a class that accepts a string which is a Email Template Name Key.

The method will run the appropriate query and get back a list, grabs the email template.

The list and template are passed to a parser to merge each of the memberships to the template and returns a list emails.

 EndOfBackgroundInfoIfYouCare

So the real question... what is the best way to do this?

One way is to just use a switch

public List<Membership> Execute(string TemplateKey) {
switch (TemplateKey) 
        {
            case "SomethingExpired":
                QueryResult = new SomethingExpiredEmailQuery().ExecuteQuery();
                break;
            case "SomethingExpireIn30":
                QueryResult = new SomethingExpireIn30EmailQuery().ExecuteQuery();
                break;
            case "FirstTimeLoginThanks":
                QueryResult = new FirstTimeLoginThanksEmailQuery().ExecuteQuery();
                break;
            case "SecurityTraining":
                QueryResult = new SecurityTrainingEmailQuery().ExecuteQuery();
                break;
            case ETC ETC ETC...

}

Another way would be to use an interface

IEmailQuery
void ExecuteQuery()

But if I use an interface I will still need to instantiate the Query class. It saves no code and does not make the code easier to maintain.

With reflection I could do something like name all of the Email queries with a pattern: Email Template Key of SecurityTraining has a query name of SecurityTrainingEmailQuery and I could use reflection to instantiate and call the ExecuteQuery method.

Without using reflection, is there no cleaner way of wiring this up?

Answer 1

One option is to have a Dictionary<string, Func<IEmailQuery>> map. You could build it like this:

private static readonly Dictionary<string, Func<IEmailQuery>> MailQueryMap = 
    new Dictionary<string, Func<IEmailQuery>> {
    { "SomethingExpired", () => new SomethingExpiredMailQuery() },
    { "SomethingExpireIn30", () => new SomethingExpireIn30EmailQuery() },
    // etc
};

Then:

public List<Membership> Execute(string templateKey) {
    IEmailQuery query = MailQueryMap[templateKey].Invoke();
    var queryResult = query.ExecuteQuery();
    // ...
}

If you can guarantee that you only ever need parameterless constructors, you could always store a Dictionary<string, Type> and instantiate it via reflection - but there will be some ugly casts etc.

EDIT: Of course, if the name of the template always is the name of the type, you could use

Type queryType = Type.GetType(namespacePrefix + "." + templateKey);
IEmailQuery query = (IEmailQuery) Activator.CreateInstance(queryType);
var queryResult = query.ExecuteQuery();

You may also want to consider using an enum instead of the magic string constants.

Answer 2

Actually, this doesn't look too smelly to me. If you don't like the switch-statement you could go the IEmailQuery-Path and just wire it up in a Dictionary<string,IEmailQuery> . This probably saves some lines of code, as you could access it like that:

QueryDictionary["MyKey"].ExecuteQuery(); 

Cheers, Oliver

Answer 3

I'd go for a Factory pattern, something like

class EmailQueryFactory
{
  public IEmailQuery Create(String TemplateKey)
  {
    ....
  }
}

and then

//.. first get String TemplateKey

IEmailQuery qry=EmailQueryFactory.Create(TemplateKey);
qry.Execute();

Answer 4

Why not using reflection as you proposed in your question? I think its a valid way of doing this kind of stuff.

Another approach would be to use the inversion of control / dependency injection pattern. You define an interface as you did and register all the known concrete implementations to your DI container (this can be done by configuration or by code).

When registering, you need to tell the DI container some service name to distinguish the implementations, because they implement the same interface.

YourIocContainer.Register<IEmailQuery>(typeof(SomethingExpiredMailQuery),
                                       "SomethingExpiredMailQuery");

When instantiating, you can get the corresponding implementation by supplying the service name again:

public List<Membership> Execute(string TemplateKey) {
   YourIocContainer.Resolve<IEmailQuery>(TemplateKey);

Answer 5

The command pattern is a perfect pattern to use for this scenario. See http://www.codeproject.com/KB/books/DesignPatterns.aspx for a practice c# description of this pattern. Lambdas as Jon Skeet has described are useful newer programming constructs that you can see. See Command Pattern : How to pass parameters to a command? for more discussion on the use of the pattern.