Factory Pattern to build many derived classes

Question

I have a factory object ChallengeManager to generate instances of a Challenge object for a game I'm building. There are many challenges. The constructors for each Challenge class derivation are different, however there is a common interface among them, defined in the base class.

When I call manager.CreateChallenge(), it returns an instance of Challenge, which is one of the derived types.

Ideally, I would like to keep the code for the object construction inside the derived class itself, so all the code related to that object is co-located. Example:

class Challenge {}

class ChallengeA : Challenge {
  public static Challenge MakeChallenge() {
    return new ChallengeA();
  }
}

class ChallengeB : Challenge {
  public static Challenge MakeChallenge() {
    return new ChallengeB();
  }
}

Now, my ChallengeManager.CreateChallenge() call only needs to decide the class to call MakeChallenge() on. The implementation of the construction is contained by the class itself.

Using this paradigm, every derived class must define a static MakeChallenge() method. However, since the method is a static one, I am not able to make use of an Interface here, requiring it.

It's not a big deal, since I can easily remember to add the correct method signature to each derived class. However, I am wondering if there is a more elegant design I should consider.

Decibel

Strongly sticking to the design you are suggesting: how will you identify which Class to instantiate in ChallengeManager.CreateChallenge()? and; if you already know the class to instantiate, why can't you just instantiate it in CreateChallange method? – Copepod

What will happen is a very long CreateChallenge() method in this case, and not as manageable as breaking out the construction to the relevant classes. – Decibel

Your design is the best here design-wise. I'd rather not put multiple static factory methods into the concrete classes, though. Use actual constructors for everything that is the classes' own responsibility and the decision making and construction of dependencies into CreateChallenge. If CreateChallenge gets too long for your taste, extract methods from it to make the intention more clear. If it's really long, extract it into its own class. YMMV – Creolacreole

Answer 1

I really like the pattern you are describing and use it often. The way I like to do it is:

abstract class Challenge 
{
  private Challenge() {} 
  private class ChallengeA : Challenge 
  {
    public ChallengeA() { ... }
  }
  private class ChallengeB : Challenge 
  {
    public ChallengeB() { ... }
  }
  public static Challenge MakeA() 
  {
    return new ChallengeA();
  }
  public static Challenge MakeB() 
  {
    return new ChallengeB();
  }
}

This pattern has many nice properties. No one can make a new Challenge because it is abstract. No one can make a derived class because Challenge's default ctor is private. No one can get at ChallengeA or ChallengeB because they are private. You define the interface to Challenge and that is the only interface that the client needs to understand.

When the client wants an A, they ask Challenge for one, and they get it. They don't need to worry about the fact that behind the scenes, A is implemented by ChallengeA. They just get a Challenge that they can use.

Assimilable

I like the idea of implementing the factory in the abstract base class itself. – Star

There are a couple downsides to this approach, though. Since ChallengeA and ChallengeB are private, you can't test them. Since you're not using abstract factory, you can't mock the factory in unit tests. Multiple factory methods for the same type means you're putting the decision of what class to create into the calling code. The beauty of a factory as far as I am concerned is that you're putting the decision of what concrete implementation to create into one single spot. This is basically legacy code by design. – Creolacreole

@Creolacreole ChallengeA and ChallengeB are meant to be used by interface Challenge. So if you get object of ChallengeA by calling Challenge.MakeA(), same for ChallengeB, you can test both. – Handtomouth

Adding partial to the class declaration can allow you to put the ChallengeA and ChallengeB in separate files in the same assembly. A worthwhile consideration if A & B are non-trivial classes. – Proudhon

Is it bad practice to make one of those classes public so I can expose methods the others shouldn't have? – Arenaceous

@AlexanderDerck: Some people consider a public nested class to be a bad practice; nested classes feel a bit weird because people aren't used to thinking of a class as something that is a member of something else (other than a namespace). So I would be cautious adapting this pattern in that way; some might find it odd. – Assimilable

@EricLippert In your example, is there any way to move each of the contained private classes (e.g., ChallengeA, ChallengeB) to their own .cs files yet still have them be partial implementations of the containing abstract class Challenge? In other words, I may A LOT of those private classes and to help with code organization, I may want to put them each in their own file in some folder in Visual Studio. Is that possible? Thank you in advance, and thank you for all you do for the C# community. – Numberless

@user1886710: But they are not partial implementations to begin with. They're subclasses. If what you're asking is whether you can say partial abstract class Challenge { private class ChallengeA ... } in one file and partial abstract class Challenge { private class ChallengeB ... in another, sure you can do that. In fact I believe we do that in the Roslyn sources. – Assimilable

@EricLippert Awesome. Got it working. Thank you for clarifying. :) – Numberless

By design, factory pattern should allow different implementations of the factory, and in your case you've merged the two sides into one, which made the decoupling principle nonexistent. Also, the return new ChallengeA(); statements are all in the abstract class itself, which implies that the abstract class knows all about its descendants. I don't think this is a good idea. – Absurdity

Answer 2

You're "decentralizing" the factory, such that each subclass is responsible for creating itself.

More commonly you would have a central factory that would know about the possible subtypes and how to construct them (often enough, simply by creating a new instance and returning that instance typed as a common interface or common base class). That approach avoids the issue you currently have. I also see no benefit to your current approach. You are currently gaining no encapsulation or code reuse over the more typical implementation of a factory.

For additional reference, have a look at

http://www.oodesign.com/factory-pattern.html

Star

ya, it's a variation of the classic scenario, mostly to contain the construction details to the derived class for readability. – Decibel

Answer 3

Not necessarily the answer you are looking for but... You can use following implementation, if you can move away from static method per class.

using System;

public class Test
{
    public static void Main()
    {
        var c1 = ChallengeManager.CreateChallenge();
        var c2 = ChallengeManager.CreateChallenge();
        //var c = ChallengeManager.CreateChallenge<Challenage>(); // This statement won't compile
    }
}

public class ChallengeManager
{
    public static Challenage CreateChallenge()
    {
        // identify which challenge to instantiate. e.g. Challenage1
        var c = CreateChallenge<Challenage1>();
        return c;
    }

    private static Challenage CreateChallenge<T>() where T: Challenage, new()
    {
        return new T();
    }
}

public abstract class Challenage{}
public class Challenage1: Challenage{}
public class Challenage2: Challenage{}

Copepod

Nice approach to use a parameterized method. – Star

With this approach, you're violating the dependency inversion principle by making the calling code depend on the concrete implementations. You gain nothing over new Challenge1() when you call CreateChallenge<Challenge1>(). The whole point of using a factory is to make classes only depend on abstractions. – Creolacreole

I agree, this one didn't really implement Factory pattern. My (updated) answer was largely centred around this statement in question: Now, my ChallengeManager.CreateChallenge() call only needs to decide the class to call MakeChallenge() on. The implementation of the construction is contained by the class itself. This way it eliminates the need to MakeChallenge method. – Copepod