Better Alternative to Case Statement

I currently have a switch statement that runs around 300 odd lines. I know this is not as giant as it can get, but I'm sure there's a better way to handle this.

The switch statement takes an Enum that is used to determine certain properties that pertain to logging. Right now the problem sets in that it is very easy to leave out an enumeration value and that it will not be given a value as it is not in the switch statement.

Is there an option one can use to ensure that every enumeration is used and gi开发者_开发知识库ven a custom set of values it needs to do its job?

EDIT:

---

Code sample as requested: (This is simplistic, but shows exactly what I mean. Also an Enumeration would exist with the below values.)

internal void GenerateStatusLog(LogAction ActionToLog)
{
    switch (ActionToLog)
    {
        case LogAction.None:
            {
                return;
            }
        case LogAction.LogThis:
            {
                ActionText = "Logging this Information";
                LogText = "Go for it.";

                break;
            }
    }

    // .. Do everything else
}

---

EDIT

I thought this over again, looked around in related questions in SO, and I wrote some code. I created a class named AdvancedSwitch<T>, which allows you to add cases and exposes a method to evaluate a value and lets you specify values that it should check for existence.

This is what I came up with:

public class AdvancedSwitch<T> where T : struct
{
    protected Dictionary<T, Action> handlers = new Dictionary<T, Action>();

    public void AddHandler(T caseValue, Action action)
    {
        handlers.Add(caseValue, action);
    }

    public void RemoveHandler(T caseValue)
    {
        handlers.Remove(caseValue);
    }

    public void ExecuteHandler(T actualValue)
    {
        ExecuteHandler(actualValue, Enumerable.Empty<T>());
    }

    public void ExecuteHandler(T actualValue, IEnumerable<T> ensureExistence)
    {
        foreach (var val in ensureExistence)
            if (!handlers.ContainsKey(val))
                throw new InvalidOperationException("The case " + val.ToString() + " is not handled.");

        handlers[actualValue]();
    }
}

You can consume the class this way:

public enum TrafficColor { Red, Yellow, Green }

public static void Main()
{
    Console.WriteLine("Choose a traffic color: red, yellow, green?");
    var color = (TrafficColor)Enum.Parse(typeof(TrafficColor), Console.ReadLine());
    var result = string.Empty;

    // Creating the "switch"
    var mySwitch = new AdvancedSwitch<TrafficColor>();

    // Adding a single case
    mySwitch.AddHandler(TrafficColor.Green, delegate
    {
        result = "You may pass.";
    });

    // Adding multiple cases with the same action
    Action redAndYellowDelegate = delegate
    {
        result = "You may not pass.";
    };
    mySwitch.AddHandler(TrafficColor.Red, redAndYellowDelegate);
    mySwitch.AddHandler(TrafficColor.Yellow, redAndYellowDelegate);

    // Evaluating it
    mySwitch.ExecuteHandler(color, (TrafficColor[])Enum.GetValues(typeof(TrafficColor)));

    Console.WriteLine(result);
}

With the creative use of anonymous delegates, you can easily add new cases to your "switch block". :)
Not that you can also use lambda expressions, and lambda blocks, eg () => { ... } instead of delegate { ... }.

You can easily use this class instead of the long switch blocks.

Original post:

If you use Visual Studio, always create swich statements with the switch code snippet. Type switch press tab twice, and it auto-generates all the possibilities for you.

Then, add a default case to the end which throws an exception, that way when testing your app you will notice that there is an unhandled case, instantly.

I mean something like this:

switch (something)
{
    ...
    case YourEnum.SomeValue:
        ...
        break;
    default:
        throw new InvalidOperationException("Default case reached.");
}

---

Well, there's throwing in the default case... There's no edit / compile time construct other than that.

However Strategy, Visitor and other patterns related to them may be appropriate if you choose to do it at run time.

Sample code will help with getting the best answer.

EDIT: Thanks for the sample. I still think it needs a bit of fleshing out as you dont cover whether there are some parameters that only apply to some cases etc.

Action is often used as an alias for the Command pattern and the fact that your Enum is called LogAction signifies that each value carries with it a behavior - be that implied (you stick appropriate code in a case) or explicit (in the specific Command hierarchy class).

Thus it looks to me like a usage of the Command pattern is appropriate (though your sample doesnt prove it) - i.e., have a class (potentially a hierarchy using constructor overloads or any other [set of] factory mechanisms) that keeps the state associated with the request along with the specific behaviour. Then, instead of passing an Enum value, create an appropriate LogCommand instance to the logger, which just invokes it (potentially passing a Log Sink 'receptacle' which the Command can log into). Otherwise you're poking random subsets of parameters in different places.

SEEALSO related posts:

• C# - Is there a better alternative than this to ‘switch on type’?

• Replace giant switch statement with what?

---

One possible solution is to use a SortedDictionary:

delegate void EnumHandler (args);
SortedDictionary <Enum, EnumHandler> handlers;

constructor
{
   handlers = new SortedDictionary <Enum, EnumHandler> ();
   fill in handlers
}

void SomeFunction (Enum enum)
{
  EnumHandler handler;

  if (handlers.TryGetValue (enum, out handler))
  {
     handler (args);
  }
  else
  {
    // not handled, report an error
  }
}

This method does allow you to replace the handlers dynamically. You could also use a List as the value part of the dictionary and have multiple handlers for each enum.

---

Try to use reflection.

• Decorate enum options with attributes that holds associated value and return this value.
• Create static class of constants and use reflection for mapping enum-option to constant by name

hope this will help

---

Some times storing the options in a map is a good solution, you can externalize the configuration to a file too, not sure if it applies to your application.

---

Long code example here, and the final generic code is a little heavy (EDIT have added an extra example that eliminates the need for the angle brackets at the expense of some final flexibility).

One thing that this solution will give you is good performance - not quite as good as a straightforward switch statement, but each case statement becomes a dictionary lookup and method invocation, so still pretty good. The first call will get a performance penalty, however, due to the use of a static generic that reflects on initialisation.

Create an attribute and generic type as follows:

[AttributeUsage(AttributeTargets.Method, AllowMultiple = false)]
public class DynamicSwitchAttribute : Attribute
{
 public DynamicSwitchAttribute(Type enumType, params object[] targets)
 { Targets = new HashSet<object>(targets); EnumType = enumType; }
 public Type EnumType { get; private set; }
 public HashSet<object> Targets { get; private set; }
}

//this builds a cache of methods for a given TTarget type, with a 
//signature equal to TAction,
//keyed by values of the type TEnum.  All methods are expected to 
//be instance methods.
//this code can easily be modified to support static methods instead.
//what would be nice here is if we could enforce a generic constraint 
//on TAction : Delegate, but we can't.
public static class DynamicSwitch<TTarget, TEnum, TAction>
{
 //our lookup of actions against enum values.
 //note: no lock is required on this as it is built when the static 
 //class is initialised.

 private static Dictionary<TEnum, TAction> _actions = 
   new Dictionary<TEnum, TAction>();

 private static MethodInfo _tActionMethod;
 private static MethodInfo TActionMethod
 {
  get
  {
   if (_tActionMethod == null)
   {
    //one criticism of this approach might be that validation exceptions
    //will be thrown inside a TypeInitializationException.
    _tActionMethod = typeof(TAction).GetMethod("Invoke", 
      BindingFlags.Instance | BindingFlags.Public);

    if (_tActionMethod == null)
     throw new ArgumentException(/*elided*/);

    //verify that the first parameter type is compatible with our 
    //TTarget type.
    var methodParams = _tActionMethod.GetParameters();
    if (methodParams.Length == 0)
     throw new ArgumentException(/*elided*/);

    //now check that the first parameter is compatible with our type TTarget
    if (!methodParams[0].ParameterType.IsAssignableFrom(typeof(TTarget)))
     throw new ArgumentException(/*elided*/);
   }
   return _tActionMethod;
  }
 }

 static DynamicSwitch()
 {
  //examine the type TTarget to extract all public instance methods 
  //(you can change this to private instance if need be) which have a
  //DynamicSwitchAttribute defined.
  //we then project the attributes and the method into an anonymous type
  var possibleMatchingMethods = 
     from method in typeof(TTarget).
       GetMethods(BindingFlags.Public | BindingFlags.Instance)
     let attributes = method.GetCustomAttributes(
        typeof(DynamicSwitchAttribute), true).
        Cast<DynamicSwitchAttribute>().ToArray()
     where attributes!= null && attributes.Length == 1 
        && attributes[0].EnumType.Equals(typeof(TEnum))
     select new { Method = method, Attribute = attributes[0] };

  //create linq expression parameter expressions for each of the 
  //delegate type's parameters
  //these can be re-used for each of the dynamic methods we generate.
  ParameterExpression[] paramExprs = TActionMethod.GetParameters().
    Select((pinfo, index) =>
    Expression.Parameter(
      pinfo.ParameterType, pinfo.Name ?? string.Format("arg{0}"))
    ).ToArray();
  //pre-build an array of these parameter expressions that only 
  //include the actual parameters
  //for the method, and not the 'this' parameter.
  ParameterExpression[] realParamExprs = paramExprs.Skip(1).ToArray();

  //this has to be generated for each target method.
  MethodCallExpression methodCall = null;

  foreach (var match in possibleMatchingMethods)
  {
   if (!MethodMatchesAction(match.Method))
    continue;

   //right, now we're going to use System.Linq.Expressions to build 
   //a dynamic expression to invoke this method given an instance of TTarget.
   methodCall = 
     Expression.Call(
       Expression.Convert(
         paramExprs[0], typeof(TTarget)
       ),  
       match.Method, realParamExprs);

   TAction dynamicDelegate = Expression.
     Lambda<TAction>(methodCall, paramExprs).Compile();

   //now we have our method, we simply inject it into the dictionary, using 
   //all the unique TEnum values (from the attribute) as the keys
   foreach (var enumValue in match.Attribute.Targets.OfType<TEnum>())
   {
    if (_actions.ContainsKey(enumValue))
     throw new InvalidOperationException(/*elided*/);

    _actions[enumValue] = dynamicDelegate;
   }
  }
 }

 private static bool MethodMatchesAction(MethodInfo method)
 {
  //so we want to check that the target method matches our desired 
  //delegate type (TAction).
  //The way this is done is to fetch the delegate type's Invoke 
  //method (implicitly invoked when you invoke delegate(args)), and 
  //then we check the return type and parameters types of that
  //against the return type and args of the method we've been passed.

  //if the target method's return type is equal to or derived from the 
  //expected delegate's return type, then all is good.

  if (!_tActionMethod.ReturnType.IsAssignableFrom(method.ReturnType))
   return false;

  //now, the parameter lists of the method will not be equal in length, 
  //as our delegate explicitly includes the 'this' parameter, whereas 
  //instance methods do not.

  var methodParams = method.GetParameters();
  var delegateParams = TActionMethod.GetParameters();

  for (int i = 0; i < methodParams.Length; i++)
  {
   if (!methodParams[i].ParameterType.IsAssignableFrom(
        delegateParams[i + 1].ParameterType))
    return false;
  }
  return true;
 }


 public static TAction Resolve(TEnum value)
 {
  TAction result;

  if (!_actions.TryGetValue(value, out result))
   throw new ArgumentException("The value is not mapped");

  return result;
 }
}

Now do this in a Unit Test:

[TestMethod]
public void TestMethod1()
{
  Assert.AreEqual(1, 
    DynamicSwitch<UnitTest1, Blah, Func<UnitTest1, int>>.
      Resolve(Blah.BlahBlah)(this));

  Assert.AreEqual(125, 
    DynamicSwitch<UnitTest1, Blah, Func<UnitTest1, int>>.
      Resolve(Blah.Blip)(this));

 Assert.AreEqual(125, 
    DynamicSwitch<UnitTest1, Blah, Func<UnitTest1, int>>.
      Resolve(Blah.Bop)(this));
}

public enum Blah
{
 BlahBlah,
 Bloo,
 Blip,
 Bup,
 Bop
}


[DynamicSwitchAttribute(typeof(Blah), Blah.BlahBlah)]
public int Method()
{
 return 1;
}

[DynamicSwitchAttribute(typeof(Blah), Blah.Blip, Blah.Bop)]
public int Method2()
{
 return 125;
}

So, given a value of TEnum, and your preferred 'action' type (in your code you would appear to be simply returning nothing and modifying the internal state of the class), you simply consult the DynamicSwitch<> class, ask it to resolve a target method, and then invoke it inline (passing the target object on which the method will be invoked as the first parameter).

I'm not really expecting any votes for this - it's a MAD solution to be honest (it does have the advantage of being able to be applied for any enum type, and even discreet values of type int/float/double, as well as supporting any delegate type) - so perhaps it's a bit of a sledgehammer!

EDIT

Once you have a static generic like this, angle-bracket hell ensues - so we want to try and get rid of them. A lot of the time, this is done by type inference on method parameters etc - but we have a problem here that we can't easily infer a delegate's signature without repeating the method call i.e. (args) => return.

However, you seem to require a method that takes no parameters and returns void, so you can close over this behemoth generic by fixing the delegate type to Action, and throw a fluid API into the mix as well (if that's your kind of thing):

public static class ActionSwitch
{
  public class SwitchOn<TEnum>
  {
    private TEnum Value { get; set; }

    internal SwitchOn(TEnum value)
    {
      Value = value;
    }

    public class Call<TTarget>{
      private TEnum Value { get; set; }
      private TTarget Target { get; set; }

      internal Call(TEnum value, TTarget target)
      {
        Value = value;
        Target = target;
        Invoke();
      }

      internal void Invoke(){
          DynamicSwitch<TTarget, TEnum, Action<TTarget>>.Resolve(Value)(Target);
      }
    }

    public Call<TTarget> On<TTarget>(TTarget target)
    {
      return new Call<TTarget>(Value, target);
    }
  }

  public static SwitchOn<TEnum> Switch<TEnum>(TEnum onValue)
  {
    return new SwitchOn<TEnum>(onValue);
  }
}

Now add this to the test project:

[TestMethod]
public void TestMethod2()
{
  //no longer have any angle brackets
  ActionSwitch.Switch(Blah.Bup).On(this);

  Assert.IsTrue(_actionMethod1Called);
}

private bool _actionMethod1Called;

[DynamicSwitch(typeof(Blah), Blah.Bup)]
public void ActionMethod1()
{
  _actionMethod1Called = true;
}

Only issue with this (apart from the complexity of the solution :) ) is that you'd have to re-build this static wrapper type whenever you want to use a new type of target delegate for a dynamic switch elsewhere. You could generate a generic version based on the Action<...> and Func<...> delegates that incorporates TArg1, TArg(n) and TReturn (if Func<>) - but you'd end up writing a lot more code.

Perhaps I'll turn this into an article on my blog and do all of that - if I get the time!