What is a correct way of replacing the switch block and Enum in this situation (C#)?

Question

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.

Answer 1

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.