5

I have two versions of a factory class designed based on the article

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

public abstract class Employee
{
    public string Name { get; set; }
    protected string Role { get; set; }
    public abstract string GetRole();
}

public class Manager : Employee
{
    public Manager()
    {
        Role = "MGR";
    }
    public override string GetRole()
    {
        return this.Role;
    }
}

Version 1: Simple, Violates Open Close Principle

Need to change SimpleEmployeeFactory every time, when I add a new concrete class

public class SimpleEmployeeFactory
{
    public static Employee GetEmployee(int typeId)
    {
        switch (typeId)
        {
            case 1:
                return new Manager();
            case 2:
                return new TechnicalLead();
            default:
                return null; //if the id doesn't have any 
        }
    }
}

Version 2:

Refactored Factory, still needs a Concrete Class creation, before we use factory call

public abstract class Employee
{
    public string Name { get; set; }
    protected string Role { get; set; }
    public abstract string GetRole();      
    public abstract Employee createEmployee();
} 

public class ChiefTechnologyOfficer : Employee
{
    public ChiefTechnologyOfficer()
    {
        this.Role = "CTO";
    }
    static ChiefTechnologyOfficer()
    {            
        RefactoredFactory.Instance.registerEmployee(5, new ChiefTechnologyOfficer());
    }
    public override string GetRole()
    {            
        return this.Role;
    }        
    public override Employee createEmployee()
    {
        return new ChiefTechnologyOfficer();
    }
}

Factory

class RefactoredFactory
{
    private static readonly RefactoredFactory instance = new RefactoredFactory();

    static RefactoredFactory()
    {
    }

    private RefactoredFactory()
    {
    }

    public static RefactoredFactory Instance
    {
        get
        {
            return instance;
        }
    }

    private Dictionary<int, Employee> registeredEmployees = new Dictionary<int, Employee>();

    public void registerEmployee(int typeId, Employee employeeInst)
    {
        registeredEmployees.Add(typeId, employeeInst);
    }

    public Employee createEmployee(int typeId)
    { 
        return ((Employee)registeredEmployees[typeId]).createEmployee();
    }
}

Client

 Employee emp = SimpleEmployeeFactory.GetEmployee(1);
 Activator.CreateInstance(typeof(ChiefTechnologyOfficer)); //Avoid
 Employee empFNoR = RefactoredFactory.Instance.createEmployee(5);

You can see Activator.CreateInstance(typeof(ChiefTechnologyOfficer)) call to make the concrete classes to register themselves with the Factory. Otherwise we cant retrieve the object

Is there a way to create a Factory class with out violating OCP principle & with out creating an object like the one I used in RefactoredFactory class?

Murali Murugesan
  • 22,423
  • 17
  • 73
  • 120
  • IMO the bigger problem is that you model roles via inheritance. – Loki Mar 11 '14 at 14:04
  • @Loki, that is an example I gave it here for Factory. Usually it will be a different concrete classes for a component like TextBox, DropDownList and all are derived from a base class Control – Murali Murugesan Mar 11 '14 at 14:19
  • The Open Closed principle applies to Client classes. Factories are not clients, i.e., they are not protected from adding new implementations. They're part of the Open part (the part that changes). It's true you can do Dynamic Factories using reflection, but there are security risks associated with that. – Fuhrmanator Mar 11 '14 at 20:45
  • @Fuhrmanator, I dont agree that OCP only applies to client. It applies to every part of the system where ever we look for loosely coupling. – Murali Murugesan Mar 12 '14 at 10:54

2 Answers2

3

It looks like the typeId suffers from Feature Envy. Instead, define a polymorphic type to capture the type; e.g. an interface:

public interface IEmployeeType
{
    Employee Create()
}

Now you can define e.g. a ManagerType, and a TechnicalLeadType, etc. Example:

public class ManagerType : IEmployeeType
{
    public Employee Create()
    {
        return new Manager();
    }
}

This is essentially an Abstract Factory, and it has the advantage that you can always create a new implementation when you need to create a new sub-type.

If you're at the boundary of a system, and must translate a primitive value like an integer to a polymorphic value, you can use one of the Role Hint patterns - particularly Metadata, Role Interface, or (my favourite) Partial Type Name.

Client

Given an IEmployeeType instance employeeType, a client would simply go:

Employee emp = employeeType.Create();
Mark Seemann
  • 225,310
  • 48
  • 427
  • 736
  • Could you please add the code for Factory and Client? I got little confusion over there. – Murali Murugesan Mar 11 '14 at 13:51
  • I've added the client code. There's no additional Factory; the `employeeType` *is* the Factory. – Mark Seemann Mar 11 '14 at 14:02
  • But `employeeType.Create()` requires some concrete class for `employeeType` in real code. ie. implementer of IEmployeeType – Murali Murugesan Mar 11 '14 at 14:17
  • Correct. That would be a class like `ManagerType` above. – Mark Seemann Mar 11 '14 at 14:41
  • But I feel like my purpose of having a Factory is lost. I don't want client to directly use a polymorphic type, ex: ManagerType. I may add a new type in future with new concrete class like `SalesExecutive`. Client need not to know about those changes. – Murali Murugesan Mar 11 '14 at 14:48
  • The client wouldn't have to reference `ManagerType`; it only needs to reference `IEmployeeType`. Then you can supply concrete implementations like `ManagerType` from other libraries. This is simply the Dependency Inversion Principle applied. – Mark Seemann Mar 11 '14 at 15:04
  • In that case IoC containers will replace my whole code. Is there to use only Factory for my case? I read [Factory Vs Dependency Injection](http://stackoverflow.com/questions/557742/dependency-injection-vs-factory-pattern) – Murali Murugesan Mar 11 '14 at 15:13
  • You don't have to use a DI Container to apply the Dependency Inversion Principle: http://blog.ploeh.dk/2012/11/06/WhentouseaDIContainer – Mark Seemann Mar 11 '14 at 15:16
0

Thala, Instead of using static constructor,register method to populate dictionary of Types. You can use config based solution like .net DbProviderFactory, to register all types.

<EmployeeFactories>
  <add name="manger"  type="Manager, EmployeeAssmbly" />
..
</EmployeeFactories>
Amit Gaikwad
  • 923
  • 1
  • 9
  • 15