views:

99

answers:

3

Hello experts, I'm having a hard time figuring out how to implement a factory pattern in a DTO mapper I'm trying to create. I'm pretty sure I need to rethink my design. Here is a very small example of what I'm running in to:

    public abstract class Person
{
    public string Name { get; set; }
    public decimal Salary { get; set; }
}

public class Employee : Person
{
    public Employee()
    {
        this.Salary = 20000;
    }

}

public class Pilot : Person
{
    public string PilotNumber { get; set; }

    public Pilot()
    {
        this.Salary = 50000;
    }
}

public static class PersonFactory
{
    public static Person CreatePerson(string typeOfPerson)
    {
        switch (typeOfPerson)
        {
            case "Employee":
                return new Employee();
            case "Pilot":
                return new Pilot();
            default:
                return new Employee();
        }
    }
}

and to use the factory:

Person thePilot = PersonFactory.CreatePerson("Pilot");
        ((Pilot)thePilot).PilotNumber = "123ABC";

How do I get around loading the pilot number without typecasting it to Pilot?? is this the wrong way to do this? I could put the pilot number in the Person class, but then Employee would inherit the number and that's not what I want. What can I do?

Thanks!

-Jackson

+6  A: 

The factory pattern is best used when the objects differ in implementation, not interface. In your case the factory pattern is not too beneficial, and you are probably better off creating your objects directly (or some other pattern maybe better).

Matt Greer
A: 

Do you have to use a string to communicate the type you want? You could use generics instead:

public static T CreatePerson<T>() where T : Person

Now it's hard to say exactly whether this will work or not, because we don't know the details of what you'd really be doing within CreatePerson. If it's just calling a parameterless constructor, that's easy:

public static T CreatePerson<T>() where T : Person, new()
{
    return new T();
}

However, that's also reasonably pointless as the caller could do that instead. Is generics viable in your actual situation? If not, could you explain why not, and we could try to work around it?

Jon Skeet
Generics may work in this case. I basically have a database table that I have a table per type hierarchy. Let's say a Person place with a "PersonType" field. and another table with "pilot" specific information. To get a result set of people if would iterate through the list of database rows, if it's a "Pilot" type, load the pilot table information, if it's an "Employee" type load that specific info. But DON"T load pilot stuff into employee objects...does that make it a bit more clear? THanks!
adminJaxon
+2  A: 

You could add methods for specific types to your PersonFactory class, or add a generic CreatePerson<T>() method, but that would only be useful if the caller already knows what type of person it should be receiving. Maybe this is the case, or maybe not.

With this scenario, I'd expect that the code that is actually making the call to PersonFactory.CreatePerson would not know or care what kind of person is being returned. If you have some code after that point that already knows or figures out what type of person object you have, then you will simply have to cast it.

Below is a code example that illustrates what you could do on your factory and different usage scenarios, attempting to explain when you simply need to cast or when you don't.

public static class PersonFactory
{
    public static Person CreatePerson()
    {
        return new Person();
    }

    public static Employee CreateEmployee()
    {
        return new Employee();
    }

    public static Pilot CreatePilot()
    {
        return new Pilot();
    }

    public static T CreatePerson<T>()
        where T : Person
    {
        return (T)CreatePerson(typeof(T));
    }

    public static Person CreatePerson(Type type)
    {
        if (type == typeof(Person))
            return CreatePerson();
        else if (type == typeof(Employee))
            return CreateEmployee();
        else if (type == typeof(Pilot))
            return CreatePilot();
        else
            throw new ArgumentOutOfRangeException(string.Format(CultureInfo.InvariantCulture, "Unrecognized type [{0}]", type.FullName), "type");
    }

    public static Person CreatePerson(string typeOfPerson)
    {
        switch (typeOfPerson)
        {
            case "Employee":
                return CreateEmployee();
            case "Pilot":
                return CreatePilot();
            default:
                return CreateEmployee();
        }
    }
}



class UsageExample
{
    Person GetPerson()
    {
        Pilot p;
        p = (Pilot)PersonFactory.CreatePerson("Pilot"); // this code already knows to expect a Pilot, so why not just call CreatePilot or CreatePerson<Pilot>()?
        p = PersonFactory.CreatePilot();
        p = PersonFactory.CreatePerson<Pilot>();
        return p;
    }

    Person GetPerson(Type personType)
    {
        Person p = PersonFactory.CreatePerson(personType);
        // this code can't know what type of person was just created, because it depends on the parameter
        return p;
    }

    void KnowledgableCaller()
    {
        Type personType = typeof(Pilot);

        Person p = this.GetPerson(typeof(Pilot));
        // this code knows that the Person object just returned should be of type Pilot

        Pilot pilot = (Pilot)p;
        // proceed with accessing Pilot-specific functionality
    }

    void IgnorantCaller()
    {
        Person p = this.GetPerson();
        // this caller doesn't know what type of Person object was just returned

        // but it can perform tests to figure it out
        Pilot pilot = p as Pilot;
        if (pilot != null)
        {
            // proceed with accessing Pilot-specific functionality
        }
    }
}
Dr. Wily's Apprentice
I like the IgnoarantCaller method. I may be able to use that! I will be loading a bunch of records from a table per type table per hierarchy. if the person has a "Pilot" in their "PersonType" column, then load the pilot number from Pilot table. if the row has a "Employee" type in the PersonType field, then don't load pilot number. ..
adminJaxon