My first stab at the Factory Pattern. Feedback please.

I am building a staffing tool and using the Factory Pattern. Can you please take a look and let me know if I did it right? Also, I'm a bit confused about why I have to create a new creator class for each type. Can't I just make one method that returns the right employee type instead of making all of these creator classes?

// abstract product
public abstract class Employee  
{  
   public abstract void Add();  
   public abstract void Update();  
   public abstract Employee Get();  
   public abstract void Delete();  
   public abstract DataTable Search();  
   public string FirstName { get; set; }  
   public string LastName { get; set; }  
   public string Title { get; set; }  
}  

// concrete product 1
public class Developer : Employee  
{  
   public Developer()  
   {  
       Title = "Software Engineer";  
   }  
   public override void Add()  
   {  
   }  
   public override void Update()  
   {  
   }  
   public override Employee Get()  
   {  
       return new Model();  
   }  
   public override void Delete()  
   {  
   }  
   public override DataTable Search()  
   {  
       throw new NotImplementedException();  
   }  
}  


// concrete product 2
public class Secretary : Employee  
{  
   public Secretary()  
   {  
       Title = "Jr. Receptionist";  
   }  
   public override void Add()  
   {  
   }  

   public override void Update()  
   {  
   }  

   public override Employee Get()  
   {  
       return new Model();  
   }  

   public override void Delete()  
   {  
   }  

   public override DataTable Search()  
   {  
       throw new NotImplementedException();  
   }  
}  

// abstract creator
public abstract class EmployeeCreator  
{  
   // factory method
   public abstract Employee CreateEmployee();  
}  

// concrete creator 1
public class DeveloperCreator : EmployeeCreator  
{  
   public override Employee CreateEmployee()  
   {  
       return new Developer();  
   }  
}  

// concrete creator 2
public class SecretaryCreator : EmployeeCreator  
{  
   public override Employee CreateEmployee()  
   {  
       return new Secretary();  
   }  
}  

// console app to test this all
static void Main(string[] args)  
{  
    EmployeeCreator employeeCreator;  
    if (args.Length > 0)  
    {  
        EmployeeType type = (EmployeeType)Enum.Parse(typeof(EmployeeType), args[0], true);  
        if (type == EmployeeType.Secretary)  
        {  
            employeeCreator = new SecretaryCreator();  
        }  
        else
        {  
            employeeCreator = new DeveloperCreator();  
        }  
    }  
    else
    {  
        throw new InvalidOperationException();  
    }  
    Employee employee = employeeCreator.CreateEmployee();  
    Console.WriteLine(employee.Title);  
    Console.ReadLine();  
}

Joe Rogers, Mar 21, 2011

Reply 1

Hi Joe,

I would prefer a single class called EmployeeCreatorFactory exposing a set of methods one for each employee type. Something like the following pseudo code:

public class EmployeeCreatorFactory
{
   public Employee createSecretary()
   {
   }

   public Employee createDeveloper()
   {
   }

   public Employee createContingentWorker()
   {
   }
}

Thanks.

Vijay Kumar, Mar 26, 2011

Reply 2

I kinda like Rajesh version using generics. Here is another version of it (with generics)... what do you think?

namespace AbstractFactoryGenerics {
    using System;
    using System.Data;

    internal class Program {
        private static void Main() {
            var developer = EmployeeFactory.Create<Developer>();
            var secretary = EmployeeFactory.Create<Secretary>();

            Console.WriteLine(string.Format("I'm a {0}", developer));
            Console.WriteLine(string.Format("I'm a {0}", secretary));
        }

        public class Developer : Employee {
            public Developer() {
                Title = "Software Engineer";
            }

            public override void Add() {}
            public override void Update() {}
            public override Employee Get() {
                throw new NotImplementedException();
            }
            public override void Delete() {}
            public override DataTable Search() {
                throw new NotImplementedException();
            }

            public override string ToString() {
                return Title;
            }
        }

        public abstract class Employee {
            public string FirstName { get; set; }
            public string LastName { get; set; }
            public string Title { get; set; }
            public abstract void Add();
            public abstract void Update();
            public abstract Employee Get();
            public abstract void Delete();
            public abstract DataTable Search();
        }

        public class Secretary : Employee
        {
            public Secretary()
            {
                Title = "Jr. Receptionist";
            }
            public override void Add() { }

            public override void Update() { }

            public override Employee Get()
            {
                throw new NotImplementedException();
            }

            public override void Delete() { }

            public override DataTable Search()
            {
                throw new NotImplementedException();
            }

            public override string ToString()
            {
                return Title;
            }
        }

        public static class EmployeeFactory {
            public static T Create <T>() {
                return (T) Activator.CreateInstance(typeof(T));
            }
        }
    }
}

Robert Blixt, Mar 24, 2011

Reply 3

Thanks, Rajesh. I made my actual implementation skip the generics and reflection. I'm not really sure if I would need them. If you find any fault with that statement, please let me know how I could benefit from using them instead of this method below. I put this method into my abstract employee class, fyi.

public static Employee EmployeeFactory(EmployeeType type)
{
             if (type == EmployeeType.Developer)
                return new Developer();
            else if (type == EmployeeType.Secretary)
                return new Secretary();

}

Joe Rogers, Mar 23, 2011

Reply 4

Hi Joe,

You are pretty right. In fact rather than creating so many creator you can use the c# language feature generics and create one creator which accepts a generic type. Your code would be more maintainable.

For sample implementation do check the following links..

http://aabs.wordpress.com/2007/11/21/a-generic-class-factory-for-c-30/

Short example here...(haven't tested this)

public class Program
    {
        // console app to test this all
        static void Main(string[] args)
        {
            EmployeeType type;

            if (args.Length > 0)
            {
                type = (EmployeeType)Enum.Parse(typeof(EmployeeType), args[0], true);
            }
            else
            {
                throw new InvalidOperationException();
            }

            Employee employee = EmployeeFactory<Employee>.Create(type);

            Console.WriteLine(employee.Title);
            Console.ReadLine();
        }
    }

    public static class EmployeeFactory<T> where T : Employee 
    {
           public static T Create(EmployeeType type) 
           {
               return (T)Activator.CreateInstance(Type.GetType("Test." + type.ToString()));            
           }
    }

NOTE:; This is a quick and dirty code. In CreateInstance I have hardcoded the "namespace". You may need to modify the code to remove this hardcoding.

This code uses reflection. Though reflection is costly but it depends of the nature of app you are building. In most cases I found the benefit overriding the cost.

Let me know your thoughts.

Hope this helps.

Rajesh Pillai, Mar 22, 2011

Reply 5

Basically you are making the subclasses to decide what to instantiate.

If you need to set specific behavoiur or properties to the instance that is created in the creater class, then this is useful. Also it lets good separation of concerns.

For instance, if "Secretary" timings are different based on the location, then those things can be set in the creater class.

Hope this helps you to understand...

Vivekananda P, Mar 21, 2011