views:

346

answers:

4

Good people of stackoverflow,

As always, I am writing a factory in order to dynamically instantiate objects.

To schematize, I have four types:

class CatDescriptor : PetDescriptor
class DogDescriptor : PetDescriptor

class Cat : Pet
class Dog : Pet

I instanciate the two last types from the factory. And here comes the dilemma: Should I just test the descriptor types with the "is" operator which hides reflection and then cost something.

static Pet.Factory(PetDescriptor descriptor)
{
    if (descriptor is CatDescriptor)
    {
        return new Cat();
    }
    else if (...)
    {
        ...
    }
}

Should I use an Enum "Type" as an attribute embedded in the PetDescriptor.

class PetDescriptor
{
    public Type PetType;

    public enum Type
    {
        Cat,
        Dog
    }
}

static Pet.Factory(PetDescriptor descriptor)
{
    switch (descriptor.PetType)
    {
        case PetDescriptor.Type.Cat:
            return new Cat();
        ....
    }
}

Or use virtual methods:

class PetDescriptor
{
    public virtual bool IsCat()
    {
        return false;
    }

    ...
}

class CatDescriptor : PetDescriptor
{
    public override bool IsCat()
    {
        return true;
    }
}

static Pet.Factory(PetDescriptor descriptor)
{
    if (descriptor.IsCat())
    {
        return new Cat();
    }
    else if (...)
    {
        ...
    }
}

Votes are opened !

edit: question is about reflection performance, and not factory design.

+1  A: 

Having tests in your factory defeat the purpose (you'll have to update your class for every new concrete instance you want to create).

You can either :

Yann Schwartz
I think you have the wrong end of the stick he's not unit testing he's creating a Pet from a pet descriptor. and The factory Uses the Pet descriptor to decide which pet to create. At least thats my understanding.
Omar Kooheji
We're not talking about Unit Testing here, but factories. IoC and abstract factories are not (only) about testing. They're about loosely coupled and maintenable design.
Yann Schwartz
Hi yann,I don't see the added value of such a pattern here... Could you please make a quick implementation with my example to convince me ? Thx.
Roubachof
A: 

Your third solution (with virtual method) is definitely out : the PetDescriptor class should not know about all it's derived classes (you would have to add a new method IsXXX every time you create a XXXDescriptor class).

I think your first solution is the best. The enum doesn't add anything useful, it just forces you to add more code in descriptor classes

Thomas Levesque
A: 

You don't want to have a single factory class you want to have factoriy classes for each Tyep of pet.

They should all implement a Pet factory interface.

The way you have it every time you add a new pet you will have to edit the Pet factory, rather than just creating a new pet factory class and using that.

Alternatively each pet class could be responsible for creating instances of itself (or the pet descriptor whcih is linked to a pet. Have a look at the Factory method pattern:

http://en.wikipedia.org/wiki/Factory_method_pattern

http://www.dofactory.com/Patterns/PatternFactory.aspx

At least thats my understanding.

Omar Kooheji
+1  A: 

Since your PetDescriptor identifies the Pet I would use overload:

static class PetFactory
{
    public static Dog CreatePet(DogDescriptor descriptor)
    {
        return new Dog(descriptor);
    }

    public static Cat CreatePet(CatDescriptor descriptor)
    {
        return new Cat(descriptor);
    }
}

(edit)

Of course, this only works if you have a concrete PetDescritor : CatDescriptor or DogDescriptor.

If you don't have the abstract PetDescriptor upon creation I would go with the first solution.

Alternately, you could declare an Enum in the Factory class that would specify with concrete Pet you like to create. The Wikipedia has a simple example with Pizza.

bruno conde