Is it OK for a factory method to return null?

Question

I'm wondering about best practice here. Is it good practice for a factory method to return null if it can't create anything? Here's an example:

ICommand command = CommandFactory.CreateCommand(args);
if (command != null)
    command.Execute();
else
    // do something else if there is no command

An alternative would be to return a NullCommand or something, I guess, but what is best practice?

Answer 1

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.

Whether you really want it to return null in this situation depends on the bigger picture, of course. (Is there a reasonable null object implementation you could return instead, for example?)

Answer 2

I agree with Jon Skeet. CreateCommand clearly implies construction.

If you won't throw an Exception, then in that scenario I would personally go with the NullCommand implementation, to avoid conditional statements in all consumers and possible NullReferenceException errors.

Answer 3

Returning null in this case will make your method harder to use; clients have to be aware of an implicit failure condition. Instead, throw an exception, and you can also provide a separate method for clients to test for this condition:

if (CommandFactory.CanCreate(args)) {
  ICommand command = CommandFactory.Create(args);
  command.Execute();
}

Or make the factory instantiatable; which would be better if you need to pre-process args:

CommandFactory factory = new CommandFactory(args);
if (factory.IsValid()) {
  ICommand command = factory.Create();
  command.Execute();
}

The interface of the factory now makes it clear and explicit that creation might fail, but it still requires the client to use the checking method. Another option is this:

ICommand command;
if (CommandFactory.TryCreate(args, out command)) {
  // creation succeeded ...
}

Answer 4

It only makes sense to return Null if there's a reason why you would want the user to have to check for null every time he calls Create. Normally you'd consider the following a perfectly valid use pattern:

var obj = MyFactory.CreateThing();
obj.DoSomething();

But what you're proposing is to force the following usage pattern:

var obj = MyFactory.CreateThing();
if (obj == Null) {
    // Handle null condition
} else {
    obj.DoSomething();
}

Normally the Null scenario would mean some sort of failure, in which case an Exception would probably make the most sense. But ultimately you're the music maker here and have to decide what is sensible in the world you're building.