16

I was writing some code over the weekend and I found myself wanting to write a factory as a static method in a base class.

My question is simply to know if this is a c# idomatic approach?

My sense that it might not be comes from the fact that the base class has knowledge of the derived class.

That said, I'm not sure of a simpler way to get the same result. A whole other factory class seems (to me at least) like unneeded complexity (?)

Something like:

class Animal
{
  public static Animal CreateAnimal(string name)
  {
     switch(name)
     {
        case "Shark":
          return new SeaAnimal();
          break;
        case "Dog":
          return new LandAnimal();
          break;
        default:
          throw new Exception("unknown animal");
     }
  }
}
class LandAnimal : Animal
{
}

class SeaAnimal : Animal
{
}
4
  • How will you test your factories?
    – user1249
    Commented Mar 5, 2012 at 17:27
  • with the code in this question, i would not. but the answer has helped inch me along the path of integrating more testing into my coding style Commented Mar 5, 2012 at 17:29
  • Consider that having pulled in both the Seaworld and the Landworld in your Animal class, all in all make it harder to handle in an isolated setting.
    – user1249
    Commented Mar 5, 2012 at 17:31
  • Won't you quickly run into cyclic dependency issues this way? Your children SeaAnimal, LandAnimal obviously need to know about its parent class Animal. But now your Animal class also needs to know about its children to return the correct subclass ?!
    – gebbissimo
    Commented Jun 16, 2021 at 6:55

5 Answers 5

14

Well, the advantage of a separate factory class is that it can be mocked out in unit tests.

But if you're not going to do that, or make it polymorphic in any other way, then a static Factory method on the class itself is OK.

2
  • thanks, could you give an example of what you are referring to when you say polymorphic any other way? Commented Mar 5, 2012 at 16:34
  • 1
    I mean that if you ever want to be able to replace one factory method with another. Mocking it for test purposes is just one of the most common examples of that.
    – pdr
    Commented Mar 5, 2012 at 17:06
18

You could use Generics to avoid the switch statement and decouple the downstream implementations from the base class as well.:

 public static T CreateAnimal<T>() where T: new, Animal
 {
    return new T();
 }

Usage:

LandAnimal rabbit = Animal.CreateAnimal();  //Type inference should should just figure out the T by the return indicated.

or

 var rabbit = Animal.CreateAnimal<LandAnimal>(); 
16
  • 2
  • 3
    @PeterK. Fowler refers to switch statements in Code Smells, but his solution is that they should be extracted to Factory classes, rather than being replaced. That said, if you are always going to know the type at development time, generics is an even better solution. But if you don't (as the original example implied) then I would argue that using reflection to avoid a switch in a factory method can be a false economy.
    – pdr
    Commented Mar 5, 2012 at 17:10
  • switch statements and if-else-if chains are the same. i use the former because of the compile time checking that forces the default case to be handled Commented Mar 5, 2012 at 17:24
  • 4
    @PeterK, in a switch statement the code is in a single place. In full-blown OO the same code may be spread over multiple, separate classes. There is a grey area where the added complexity of having numerous snippets is less desirable than a switch-statement.
    – user1249
    Commented Mar 5, 2012 at 17:29
  • 1
    in an OOP sense, using the generic version is probably a LOT worse than the switch case, because the generic solution FORCES you to have a parameterless public constructor on all your subclasses, and this is a design smell ten times worse than any switch case (especially in a factory, arguably the one place where switch cases are actually desireable)
    – sara
    Commented Apr 14, 2016 at 19:36
5

Taken to the extreme, the factory could be generic as well.

interface IFactory<K, T> where K : IComparable
{
    T Create(K key);
}

Then one can create any type of factory of objects, which in turn could create any type of object. I am not sure if that is simpler, it is certainly more generic.

I don't see anything wrong with a switch statement for a small factory implementation. Once you are into large number of objects or possible different class hierarchies of objects, I think a more generic approach is better suited.

0
1

Bad idea. First, it violates the open-closed principle. For any new animal you would have to mess with your base class again and you would potentially break it. The dependencies would go the wrong way.

If you need to create animals from a configuration, a construct like this would be sort of OK, although using reflection to get the type that matches the name and instantiate it using that obtained type information would be a better option.

But you should create a dedicated factory class anyway, untied from the animal class hierarchy, and return an IAnimal interface rather than a base type. Then it would becomes useful, you would have achieved some decoupling.

0

This question is timely for me - I was writing almost exactly such code yesterday. Just replace "Animal" with whatever is relevant in my project, though I'll stick with "Animal" for the sake of discussion here. Instead of a 'switch' statement, I had a somewhat more complex series of 'if' statements, involving more than just comparing one variable to certain fixed values. But that's a detail. A static factory method seemed like a neat way to design things, as the design emerged from refactoring earlier quick-and-dirty messes of code.

I rejected this design on the grounds of the base class having knowledge of the derived class. If classes LandAnimal and SeaAnimal are small, neat, and easy, they can be in the same source file. But I have large messy methods for reading text files not conforming to any officially defined standards - I want my LandAnimal class in its own source file.

That leads to circular file dependency - LandAnimal is derived from Animal, but Animal needs to already know that LandAnimal, SeaAnimal and fifteen other classes exist. I pulled the factory method out, put it into its own file (in my main application, not my Animals library) Having a static factory method seemed cute and clever, but I realized that it didn't actually solve any design problem.

I have no idea how this relates to idiomatic C#, since I switch languages a lot, I usually ignore idioms and conventions peculiar to languages and dev stacks outside my usual work. If anything my C# might look "pythonic" if that's meaningful. I aim for general clarity.

Also, I don't know if there'd an advantage to using the static factory method in the case of small, simple classes unlikely to be extended in future work - having it all in one source file might be nice in some cases.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.