Reputation: 45
I'm working with a class that contains several variants of a private method. Currently, I'm using an enum to select the appropriate one within an exposed method like so.
public class MyClass
{
public enum MyEnum { Type1, Type2, Type3, Type4 };
private MyEnum _type;
public MyClass(MyEnum type)
{
Type = type;
}
public MyEnum Type
{
get { return _type; }
set { _type = value; }
}
public int Function(int x, int y)
{
switch(_type)
{
case MyEnum.Type1:
return Function1(x,y);
case MyEnum.Type2:
return Function2(x,y);
case MyEnum.Type3:
return Function3(x, y);
case MyEnum.Type4:
return Function4(x, y);
}
}
private int Function1(int x, int y)
{
// function variant 1
}
private int Function2(int x, int y)
{
// function variant 2
}
private int Function3(int x, int y)
{
// function variant 3
}
private int Function4(int x, int y)
{
// function variant 4
}
}
This works fine but I'm wondering if I'd be better off going with a private delegate which is updated whenever the enum changes. Especially since, in this case, the public method will be called much more frequently than the enum setter.
public class MyClass
{
public enum MyEnum { Type1, Type2, Type3, Type4 };
private Func<int, int, int> _function;
private MyEnum _type;
public MyClass(MyEnum type)
{
Type = type;
}
public MyEnum Type
{
get { return _type; }
set
{
_type = value;
OnTypeChange();
}
}
private void OnTypeChange()
{
switch (_type)
{
case MyEnum.Type1:
_function = Function1;
return;
case MyEnum.Type2:
_function = Function2;
return;
case MyEnum.Type3:
_function = Function3;
return;
case MyEnum.Type4:
_function = Function4;
return;
}
}
public int Function(int x, int y)
{
return _function(x, y);
}
private int Function1(int x, int y)
{
// function variant 1
}
private int Function2(int x, int y)
{
// function variant 2
}
private int Function3(int x, int y)
{
// function variant 3
}
private int Function4(int x, int y)
{
// function variant 4
}
}
I suppose I'm just looking for some conventional wisdom on the subject. How is this sort of thing typically handled out in the wild?
Upvotes: 3
Views: 3265
Reputation: 1378
If you really don't want to subclass, another alternative is to use a lookup table (LUT):
delegate int FunctionDelegate(int x, int y);
enum FunctionName { Function1, Function2, Function3 };
class FunctionItem
{
public FunctionItem(FunctionName functionName, FunctionDelegate del)
{
FunctionName = functionName;
Delegate = del;
}
public FunctionName FunctionName
{
private set;
get;
}
public FunctionDelegate Delegate
{
private set;
get;
}
}
static readonly FunctionItem[] LUT = new FunctionItem[]
{
new FunctionItem(FunctionName.Function1, Method1),
new FunctionItem(FunctionName.Function2, Method2),
new FunctionItem(FunctionName.Function3, Method3)
};
// Fragment of lookup:
FunctionItem f = LUT[function];
Debug.Assert(f.Function == function);
int result = f.Delegate(...); // Invoke delegate
Not complete, but you get the idea. As an extra advantage, you can extend FunctionItem
with extra properties like Name
, Description
, etc. per function.
As for a proper OO alternative, look into the Strategy design pattern (= subclassing).
And it's funny, because subclassing works the same... the vtable is actually a LUT, and FunctionItem becomes an abstract base class.
Upvotes: 0
Reputation: 11273
Along the lines of what BradleyDotNET is getting at, there are ways to adhere to the open/closed principle along with good design practices leveraging the power of .NET. Here is a simple example of how you could implement what you are asking for, have it be easily extensible, and easily maintainable.
public enum MyEnum
{
Value1, Value2, Value3, Value4
}
[AttributeUsage(AttributeTargets.Class, Inherited = false)]
public class MyClassHandlerAttribute : Attribute
{
public MyEnum Handles { get; private set; }
public MyClassHandlerAttribute(MyEnum handles) { Handles = handles; }
}
public abstract class MyClass
{
public abstract int Function(int x, int y);
}
public static class MyClassFactory
{
public static MyClass Create(MyEnum type)
{
var handler = Assembly.GetExecutingAssembly().GetTypes().Where(t =>
{
var a = t.GetCustomAttribute<MyClassHandlerAttribute>();
if (a != null && a.Handles == type)
return true;
return false;
}).FirstOrDefault();
if (handler != null)
return Activator.CreateInstance(handler) as MyClass;
return null;
}
}
[MyClassHandler(MyEnum.Value1)]
public sealed class MyClassType1 : MyClass
{
public int Function(int x, int y) { return x * y; }
}
[MyClassHandler(MyEnum.Value2)]
public sealed class MyClassType2 : MyClass
{
public int Function(int x, int y) { return x * x + y; }
}
You could even make the subclasses have internal default constructors so nobody outside of your assembly creates types of any class or subclass.
If you want a walk-through of the code, you have the MyEnum
type (which could be moved inside the MyClass
, but would violate OCP if you ever had to add to it), so its defined outside the class.
You then have a custom attribute MyClassHandlerAttribute
so you can mark handlers to be "auto-discovered" by the factory class.
Then you have the factory which uses type inspection to load handlers for your enum. Its pretty simple, it looks at all the types in the assembly with the custom attribute and if it handles that enum value, it returns it. (Somebody probably has a slicker LINQ query for this but I did this quick)
And then you have your sub classes, easily maintainable, no code smell, simple short code units.
Upvotes: 1
Reputation: 61339
Your second option is technically better, because you don't have to go through the switch every time the public method is called.
The first is a bit more readable.
In all reality though, switching behavior on an enum is a decent sized red flag. Usually you would subclass MyClass
and use polymorphism to get the desired behavior. I would definitely consider doing so in your case. "In the wild", that's probably the approach I would use.
Upvotes: 2