What if Your Factory Method Can’t Create Anything?

The Problem

I’ve come across a situation that had code that looked like this:

if (context.Request["cmd"] == "show-standard-stats")
    // show standard stats
else if (context.Request["cmd"] == "show-other-stats")
    // show the other stats
else if (context.Request["cmd"] == "show-some-other-stats")
    // show some other stats
else
    // don't show any stats but do something else altogether

Each of the commands does something similar with regard to displaying stats, while the code in the else block does something else entirely. I saw this code as a candidate for refactoring using the command pattern and the factory method pattern. The trouble is in the final else clause, because what’s executed there is arguably not a “show stats” command at all. My first idea was to do this:

IStatsCommand command = StatsCommandFactory.CreateCommand(context);

if (command != null)
    command.Execute();
else
    // don't show any stats but do something else altogether

A Proposed Solution

However, returning null from a factory method sounded weird to me, so I asked the StackOverflow community whether it was a good idea. Jon Skeet replied with,

I think it’s potentially reasonable for a factory method to return null in some situations, but not if it’s a method called CreateCommand. If it were GetCommand or FetchCommand, that might be okay… but a Create method should throw an exception on failure, I would suggest.

Fair enough. So I started going with that:

try
{
    IStatsCommand command = StatsCommandFactory.CreateCommand(context);
    command.Execute();
}
catch (CreationFailedException)
{
    // don't show any stats but do something else altogether
}

Another Solution

This is better, but adding in a trycatch made me uneasy because it makes the caller have to know about CreationFailedException. Maybe that’s OK, though. Another responder at StackOverflow responded that I could try this:

IStatsCommand command;
if (StatsCommandFactory.TryCreateCommand(context, out command))
{
    // creation succeeded ...
}

Now, “out” parameters make me skittish, but this looks to me like it might be a little better than surrounding the factory method call with trycatch. I’m not sure which solution I like better.

blog comments powered by Disqus