5

If it helps, the following question is in the context of a game I am building.

In a few different places I have the following scenario. There exists a parent class, for this example called Skill, and I have a number of sub-classes implementing the methods from the parent class. There also exists another parent class that we will call Vocation. The skills need to be listed in different sub-classes of Vocation. However, those skills need to be available for anything in the game that uses any given vocation.

My current setup is to have an Enum called Skill.Id, so that Vocation contains a collection of values from that Enum and when an entity in the game takes on that Vocation the collection is passed into another class, called SkillFactory. Skill.Id needs a new entry every time I create a new Skill sub-class, as well as a case in the switch block for the new sub-classes' constructor.

i.e.:

//Skill.Id
Enum{FireSkill,WaterSkill,etc}

//SkillFactory
public static Skill Create(Skill.Id id)
{
    switch(id)
    {
        case Skill.Id.FireSkill:
             return new FireSkill();
        //etc
    }
}

This works perfectly fine, but using the enum and switch block as a go between feels like more overhead than I need to solve this problem. Is there a more elegant way to create instances of these Skill sub-classes, but still allows Vocation to contains a collection identifying the skills it can use?

Edit: I am fine throwing out the enum and associated switch block, so long as Vocation can contain a collection that allows arbitrary instantiation of the Skill sub-classes.

4
  • Switch and logical if statements are extremely fast besides almost every factory implementation (I've seen) does the same thing using a switch. IMHO I wouldn't change it. Sep 9, 2011 at 0:06
  • I'd agree with this, provided you aren't duplicating the switch amongst multiple routines within the type. At that point, a Dictionary lookup is often a better approach, as you can put the "switch" in a single location in the code. Sep 9, 2011 at 0:11
  • this is the fastest and the most readable way, why should find another way?
    – ktutnik
    Sep 9, 2011 at 0:12
  • I'm not worried about any overhead performance-wise with the switch block and enum. I was worried that the duplication of information (a new sub-class implementation required not only the implementation, but also a new Skill.Id entry and subsequent SkillFactory case statement) was a sign that my approach could be better.
    – XBigTK13X
    Sep 9, 2011 at 0:13

2 Answers 2

8

You can make a Dictionary<Skill.Id, Func<Skill>> and use it to instantiate.

In the constructor:

Dictionary<Skill.Id, Func<Skill>> creationMethods = new Dictionary<Skill.Id, Func<Skill>>();
public SkillFactory()
{
     creationMethods.Add(Skill.Id.FireSkill, () => new FireSkill());
     creationMethods.Add(Skill.Id.WaterSkill, () => new WaterSkill());
}

Then, your Create method becomes:

public static Skill Create(Skill.Id id)
{
     return creationMethods[id]();
}

Granted, this isn't much better - except that it does allow you to extend this to other functionality that's per ID without duplicating the switch block if that becomes a requirement. (Just put more into the value side of the Dictionary.)

That being said, in the long run, getting rid of the enum entirely can be a good benefit for extensibility. This will require a more elaborate change, however. For example, if you used MEF, you could import a set of SkillFactory types at runtime and associate them to a name (via metadata) via a single ImportMany. This would allow you to add new Skill subclasses without changing your factory, and refer to them by name or some other mechanism.

0

if this creation function is going to be so used that a "case" will produce overhead, dictionary with enums keys will generate a lot of garbage.

In the context of a xna game, it can be worse than the "case".

"If you use an enum type as a dictionary key, internal dictionary operations will cause boxing. You can avoid this by using integer keys, and casting your enum values to ints before adding them to the dictionary." Extracted from here

You can use a simple array and cast enum to int for indexing:

 Enum {FireSkill=0,WaterSkill=1,etc}

 Func<Skill>[] CreationMethods = new Func<Skill>() 
 {
     () => new FireSkill(),
     () => new WaterSkill(),
 } 

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.