views:

64

answers:

4

Let's say I am creating a sports game, and in this game there are various positions a player can play, attack, defence, etc. So I start by creating a base class:

public abstract class Position
{
 public abstract string Name
 {
  get;
 }
}

and the subclasses...

public class Defender : Position
{
 public override string Name
 {
  get { return "Defender"; }
 }
}

and so on. This is all fine.

But now I need a function to create these objects. I need a create by position function. So one possible solution is to create an enum of all the positions and pass this value to a function that switches on the enum and returns the appropriate object. But this triggers my code smell alarm. This soultion ties together the classes, the enum and the switch inside a function:

public static Position GetByType(Types position)
{
 switch(position)
 {
  case Types.Defender:
   return new Defender();
... and further terrible code

What solution should I be looking at? Which design pattern is this?

+2  A: 

Sounds like the Factory pattern.

However it may not be a bad thing to have a switch case, which switches on an enum/string to return the right type of object.... as long as it is isolated in one place.

Gishu
+1  A: 

Indeed, what you want is an abstract factory. The ease of implementing the factory depends on the language you are using. php allows for variable class names, for example, so you can just send in the class name and get new $classname back. Other languages do not allow this, however. In fact, if your language does not do this, you have already created a factory class!

There is not really anything more elegant you can do unless you want to use reflection to simulate what php does.

tandu
+1  A: 

One way to deal with the switch is to have the factory declare an array of Types and then use the enumeration as an index into the array like so:

public abstract class Position {
    public abstract string Name {
        get;
    }
}
public class Defender : Position {
    public override string Name {
        get { return "Defender"; }
    }
}
public class Attacker : Position {
    public override string Name {
        get { return "Attacker"; }
    }
}
public static class PositionFactory {
    public enum Types {
        Defender, Attacker
    }
    private static Type[] sTypes = new Type[] { typeof(Defender), typeof(Attacker)};
    public static Position GetByType(Types positionType) {
        return Activator.CreateInstance(sTypes[(Int32)positionType]) as Position;
    }
}
Steve Ellinger
Thanks for the code! Very interesting.
DanDan
+1  A: 

If you have to do this on a small scale, the switch isn't really bad, especially if it lives in a single place.

If you have to do this on a medium scale, you might want to consider improving the internals a bit -- Steve Ellinger's suggestion is a reasonable one. Personally I prefer to use a IDictionary<MyEnum, Action<T>> where the Action returns a new instance of the class in question.

If you have to do this on a grand or configurable scale, you should probably check out an IoC controller such as structuremap or ninject or whatever the cool kids are [laying with these days.

Wyatt Barnett
For now I am using a dictionary. In the future I would like this stage configurable so this answer is going to help me out the most, thank you!
DanDan