Only allow Factory method to instantiate objects (prevent instantiation of base class AND uninitialized objects)

Question

I have a base class for handling "jobs". A factory method creates derived "job handler" objects according to job type and ensures the job handler objects are initialized with all the job information.

Calling factory method to request a handler for Job and Person assigned:

public enum Job { Clean, Cook, CookChicken }; // List of jobs.

  static void Main(string[] args)
  {
    HandlerBase handler;
    handler = HandlerBase.CreateJobHandler(Job.Cook, "Bob");
    handler.DoJob();
    handler = HandlerBase.CreateJobHandler(Job.Clean, "Alice");
    handler.DoJob();
    handler = HandlerBase.CreateJobHandler(Job.CookChicken, "Sue");
    handler.DoJob();
  }

The Result:

Bob is cooking.
Alice is cleaning.
Sue is cooking.
Sue is cooking chicken.

Job handler classes:

public class CleanHandler : HandlerBase
{
  protected CleanHandler(HandlerBase handler) : base(handler) { }
  public override void DoJob()
  {
    Console.WriteLine("{0} is cleaning.", Person);
  }
}

public class CookHandler : HandlerBase
{
  protected CookHandler(HandlerBase handler) : base(handler) { }
  public override void DoJob()
  {
    Console.WriteLine("{0} is cooking.", Person);
  }
}

A sub-classed Job Handler:

public class CookChickenHandler : CookHandler
{
  protected CookChickenHandler(HandlerBase handler) : base(handler) { }
  public override void DoJob()
  {
    base.DoJob();
    Console.WriteLine("{0} is cooking chicken.", Person);
  }
}

The best way of doing things? I have struggled with these issues:

1. Ensure that all derived objects have a fully initialized base object (Person assigned).
2. Prevent instantiation of ANY objects other than through my factory method that performs all the initialization.
3. Prevent instantiation of the base class object.

The Job handler HandlerBase base class:

1. A Dictionary<Job,Type> maps Jobs to Handler classes.
2. PRIVATE setter for job data (i.e., Person) prevents access except by factory method.
3. NO default constructor and PRIVATE constructor prevents construction except by factory method.
4. A protected "copy constructor" is the only non-private constructor. One must have an instantiated HandlerBase to create a new object, and only the base class factory can create a base HandlerBase object. The "copy constructor" throws an exception if one attempts to create new objects from a non-base object (again, preventing construction except by the factory method).

A look at the base class:

public class HandlerBase
{
  // Dictionary maps Job to proper HandlerBase type.
  private static Dictionary<Job, Type> registeredHandlers =
    new Dictionary<Job, Type>() {
      { Job.Clean, typeof(CleanHandler) },
      { Job.Cook, typeof(CookHandler) },
      { Job.CookChicken, typeof(CookChickenHandler) }
    };

  // Person assigned to job. PRIVATE setter only accessible to factory method.
  public string Person { get; private set; }

  // PRIVATE constructor for data initialization only accessible to factory method.
  private HandlerBase(string name) { this.Person = name; }

  // Non-private "copy constructor" REQUIRES an initialized base object.
  // Only the factory method can make a HandlerBase object.
  protected HandlerBase(HandlerBase handler)
  {
    // Prevent creating new objects from non-base objects.
    if (handler.GetType() != typeof(HandlerBase))
      throw new ArgumentException("THAT'S ILLEGAL, PAL!");

    this.Person = handler.Person; // peform "copy"
  }

  // FACTORY METHOD.
  public static HandlerBase CreateJobHandler(Job job, string name)
  {
    // Look up job handler in dictionary.
    Type handlerType = registeredHandlers[job];

    // Create "seed" base object to enable calling derived constructor.
    HandlerBase seed = new HandlerBase(name);

    object[] args = new object[] { seed };
    BindingFlags flags = BindingFlags.Instance | BindingFlags.NonPublic;

    HandlerBase newInstance = (HandlerBase)Activator
      .CreateInstance(handlerType, flags, null, args, null);

    return newInstance;
  }

  public virtual void DoJob() { throw new NotImplementedException(); }
}

A look at the Factory Method:

Because I have made public construction of a new object impossible without already having an instantiated base object, the factory method first constructs a HandlerBase instance as a "seed" for calling the needed derived class "copy constructor".

The factory method uses Activator.CreateInstance() to instantiate new objects. Activator.CreateInstance() looks for a constructor that matches the requested signature:

The desired constructor is DerivedHandler(HandlerBase handler), thus,

1. My "seed" HandlerBase object is placed in object[] args.
2. I combine BindingFlags.Instance and BindingFlags.NonPublic so that CreateInstance() searches for a non-public constructor (add BindingFlags.Public to find a public constructor).
3. Activator.CreateInstance() instantiates the new object which is returned.

What I don't like...

Constructors are not enforced when implementing an interface or class. The constructor code in the derived classes is mandatory:

protected DerivedJobHandler(HandlerBase handler) : base(handler) { }

Yet, if the constructor is left out you don't get a friendly compiler error telling you the exact method signature needed: "'DerivedJobHandler' does not contain a constructor that takes 0 arguments".

It is also possible to write a constructor that eliminates any compiler error, instead--WORSE!--resulting in a run-time error:

protected DerivedJobHandler() : base(null) { }

I do not like that there is no means of enforcing a required constructor in derived class implementations.

Answer 1

I see three responsibilities in your HandlerBase that, if decoupled from one another, may simplify the design problem.

1. Registration of handlers
2. Construction of handlers
3. DoJob

One way of reorganizing this would be to put #1 and #2 on a factory class, and #3 on a class with an internal constructor so that only the factory class can call it per your internal requirements. You can pass in the Person and Job values directly rather than letting the constructor pull them from a different HandlerBase instance, which would make the code easier to understand.

Once these responsibilities are separated, you can then more easily evolve them independently.

Answer 2

Whenever you say "I want to enforce x, but the code won't do it for me" Then a unit test is generally the answer.

Then the next person who comes in to make a handler will look at the tests and know what he needs to do to conform. You could even write a unit test that looped over that dictionary of registered handlers and constructed each one. Failure to create the right constructor for a new one would fail that test.