11

I'm trying to find a solution for this problem. This is my example code:

class Program
{
  private string Command;

  private static string[] Commands = { "ComandOne", "CommandTwo", "CommandThree", "CommandFour" };


  static void Main(string[] args)
  {
    Command = args[0];
    switch(Command)
    {
      case Commands[0]: //do something 
        break;
      case Commands[1]: //do something else
        break;
      case Commands[2]: //do something totally different
        break;
      case Commands[3]: //do something boring
        break;
      default: //do your default stuff
        break;
    }
  }

  void DifferentMethod()
  {
    foreach(string c in Commands)
    {
      //do something funny
    }
  }
}

This code doesn't work because the string values in the switch are not constants. I want to write easy maintainable code.
I like to use something like an array because I need to use the same values somewhere else in a loop.
With int-values an enum would be perfect, but I didn't find a small solution for the same thing with strings.

3
  • 1
    Most solutions suggest enumerations, but enumeration names have special naming requirements. If this causes problems, you can bind a DescriptionAttribute to each enumeration item to contain a friendly names (which could have spaces or whatever), and could look up those names when iterating over the enumeration within DifferentMethod.
    – Brian
    Oct 1, 2010 at 14:48
  • @Brian, good point, and to get that attribute, you will need the field: FieldInfo enumField = typeof(Commands).GetField(enumValue.ToString());
    – Kirk Woll
    Oct 1, 2010 at 19:00
  • @Kirk Woll: The next two lines after that being: DescriptionAttribute da = (DescriptionAttribute)Attribute.GetCustomAttribute(enumField, typeof(DescriptionAttribute)); if (da.Description != null) description = da.Description;
    – Brian
    Oct 1, 2010 at 19:25

9 Answers 9

19

Convert Commands into an enum:

enum Commands { ComandOne, CommandTwo, CommandThree, CommandFour }

Switch statement should look like:

static void Main(string[] args)
{
    Command = (Commands)Enum.Parse(typeof(Commands), args[0]);
    switch(Command)
    {
        case Commands.CommandOne: 
            //do something 
            break;
        case Commands.CommandTwo: 
            //do something else
            break;
        ...
        default:
            // default stuff
    }
}

And your last method should look like:

void DifferentMethod()
{
    foreach(var c in Enum.GetValues(typeof(Commands)))
    {
        string s = c.ToString(); 
        //do something funny
    }
}
5
  • 1
    i recommend to pre-calculate Enum.GetValues(typeof(Commands)) and store somewhere. it is not cheap operation.
    – Andrey
    Oct 1, 2010 at 14:25
  • 1
    @Andrey: I disagree. DifferentMethod() only takes about a dozen or so microseconds each call on my machine. Precalculating the enumeration is worth doing if profiling finds it as a bottleneck that needs fixing, and I'd be suspicious if the proper solution was to have a static precalculated copy of the enumeration. I'd rather save such optimizations until necessary (i.e. you profiled and this was the bottleneck); it makes the code slightly more complicated to have commands stored both as an enumeration and as a collection of strings.
    – Brian
    Oct 1, 2010 at 14:41
  • Isn't Enum.Parse() internally doing a switch on the string? Performance wise, calling that method alone would be just as expensive as the original method.
    – manixrock
    Oct 1, 2010 at 15:18
  • @manixrock, on the off chance your comment is directed towards me, I want to assure you that my answer and suggestion was not intended as a micro-optimization to improve the performance of the OP's code. It was for clarity and static type safety.
    – Kirk Woll
    Oct 1, 2010 at 15:21
  • @Brian agree. i should have sad: if it is bottleneck then precalc it.
    – Andrey
    Oct 1, 2010 at 16:04
8

An easy fix in your specific example:

switch(Array.IndexOf(Commands, Command))
{
    case 0: ...  
    case 1: ...

    default: //unknown command. Technically, this is case -1
} 

Other alternatives:

  1. Inline the strings.

    switch(Command) { case "CommandOne": ... case "CommandTwo": ... }

  2. Use an enumeration instead, as KirkWoll says. This is probably the cleanest solution.

  3. In more complex scenarios, using a lookup such as Dictionary<string, Action> or Dictionary<string, Func<Foo>> might provide better expressibility.

  4. If the cases are complex, you could create an ICommand interface. This will require mapping the command-string to the right concrete-implementation, for which you use simple constructs (switch / dictionaries) or fancy reflection (find ICommand implementations with that name, or with a certain attribute decoration).

1
  • 3
    +1. The OP is probably better off with Kirk's solution, but this is closer to showing the way to do exactly what he is asking for.
    – Brian
    Oct 1, 2010 at 14:45
5

Just yesterday i created a solution for it. In your case enums are better but here is my solution for general non-const switch-case situation.

usages:

    static string DigitToStr(int i)
    {
        return i
            .Case(1, "one")
            .Case(2, "two")
            .Case(3, "three")
            .Case(4, "four")
            .Case(5, "five")
            .Case(6, "six")
            .Case(7, "seven")
            .Case(8, "eight")
            .Case(9, "nine")
            .Default("");
    }

        int a = 1, b = 2, c = 3;
        int d = (4 * a * c - b * 2);
        string res = true
            .Case(d < 0, "No roots")
            .Case(d == 0, "One root")
            .Case(d > 0, "Two roots")
            .Default(_ => { throw new Exception("Impossible!"); });

        string res2 = d
            .Case(x => x < 0, "No roots")
            .Case(x => x == 0, "One root")
            .Case(x => x > 0, "Two roots")
            .Default(_ => { throw new Exception("Impossible!"); });

        string ranges = 11
            .Case(1, "one")
            .Case(2, "two")
            .Case(3, "three")
            .Case(x => x >= 4 && x < 10, "small")
            .Case(10, "ten")
            .Default("big");

definition:

class Res<O, R>
{
    public O value;
    public bool succ;
    public R result;

    public Res()
    {

    }

    static public implicit operator R(Res<O, R> v)
    {
        if (!v.succ)
            throw new ArgumentException("No case condition is true and there is no default block");
        return v.result;
    }
}

static class Op
{
    static public Res<O, R> Case<O, V, R>(this Res<O, R> o, V v, R r)
    {
        if (!o.succ && Equals(o.value, v))
        {
            o.result = r;
            o.succ = true;
        }
        return o;
    }

    static public Res<O, R> Case<O, V, R>(this O o, V v, R r)
    {
        return new Res<O, R>()
        {
            value = o,
            result = r,
            succ = Equals(o, v),
        };
    }

    static public Res<O, R> Case<O, R>(this Res<O, R> o, Predicate<O> cond, R r)
    {
        if (!o.succ && cond(o.value))
        {
            o.result = r;
            o.succ = true;
        }
        return o;
    }

    static public Res<O, R> Case<O, R>(this O o, Predicate<O> cond, R r)
    {
        return new Res<O, R>()
        {
            value = o,
            result = r,
            succ = cond(o),
        };
    }

    private static bool Equals<O, V>(O o, V v)
    {
        return o == null ? v == null : o.Equals(v);
    }

    static public R Default<O, R>(this Res<O, R> o, R r)
    {
        return o.succ
            ? o.result
            : r;
    }

    static public R Default<O, R>(this Res<O, R> o, Func<O, R> def)
    {
        return o.succ ? o.result : def(o.value);
    }
}
5

You could eliminate the switch statement altogether by creating IYourCommand objects and loading them into a Dictionary<string, IYourCommand>.

class Program
{
  private Dictionary<string, IYourCommand> Command = new Dictionary<string, IYourCommand>
    {
       { "CommandOne",   new CommandOne()   },
       { "CommandTwo",   new CommandTwo()   },
       { "CommandThree", new CommandThree() },
       { "CommandFour",  new CommandFour()  },
    };

  public static void Main(string[] args)
  {
    if (Command.ContainsKey(args[0]))
    {
      Command[args[0]].DoSomething();
    }
  }
}

public interface IYourCommand
{
  void DoSomething();
}
3
  • @FrustratedWithFormsDesigner: No, no. ICommand in this context is a mythical interface. You will have to create that interface and give it whatever name you prefer. Oct 1, 2010 at 16:02
  • @FrustratedWithFormsDesigner: I edited my answer to make it more clear. Oct 1, 2010 at 16:08
  • Ah ok. I thought there was something new and cool out there. Still, I like this design. Really seems the best choice for the OP's specific problem. Oct 1, 2010 at 16:12
3

I generally dislike strings for this sort of thing - it's too easy to get into trouble with misspellings, different casings and the like - but presumably that's why you want to use a variable instead of string literal. If the enum solutions aren't suitable, using consts should accomplish your goal.

EDIT: Oct 28, 2013 to fix an incorrect assignment

class Program
{
    private string Command;

    const string command1 = "CommandOne";
    const string command2 = "CommandTwo";
    const string command3 = "CommandThree";
    const string command4 = "CommandFour";

    private static string[] Commands = { command1, command2, command3, command4 };

    static void Main(string[] args)
    {
        string Command = args[0];
        switch (Command)
        {
            case command1: //do something 
                break;
            case command2: //do something else
                break;
            case command3: //do something totally different
                break;
            case command4: //do something boring
                break;
            default: //do your default stuff
                break;
        }
    }

    void DifferentMethod()
    {
        foreach (string c in Commands)
        {
            //do something funny
        }
    }
}
0
2

Define a Dictionary<string, enum> and map the input command to the appropriate value before entering the switch. If match is not found, then default processing happens.

2

As you said, only constant expressions are allowed in a switch. You would normally do this by defining an enum and use that in your switch.

class Program
{
  private enum Command
  {
    CommandOne = 1,
    CommandTwo = 2,
    CommandThree = 3
  }

  static void Main(string[] args)
  {
    var command = Enum.Parse(typeof(Commands), args[0]);
    switch(command )
    {
      case Command.CommandOne: //do something 
        break;
      case Command.CommandTwo: //do something else
        break;
      case Command.CommandThree: //do something totally different
        break;
      default: //do your default stuff
        break;
    }
  }
}

Use Enum.GetValues to enumerate through enum values in DifferentMethod.

4
  • +1 Enums are certainly the way to go for collections of like preset values.
    – BoltClock
    Oct 1, 2010 at 14:20
  • @Heinzi: As Kirk indicates, that's what Enum.GetValues is for.
    – Brian
    Oct 1, 2010 at 14:43
  • @digEmAll, I changed it to parse instead of a cast, although mainly the answer was to demonstrate the use of an enum. I don't think we have to spell everything out, people can think for themselves as well.
    – Carvellis
    Oct 1, 2010 at 14:51
  • 1
    You're right people can think for themselves, but it is not unusual to read questioner comments like: "I tried the code but it doesn't compile". So I always prefer to write code that at least compiles ;)
    – digEmAll
    Oct 1, 2010 at 14:58
1

You can do it the other way around and reach your objective.

Use Enum and its GetNames call to get a string array to loop through.

Enum.GetNames(typeof (*YOURENUM*));

For more info. http://msdn.microsoft.com/en-us/library/system.enum.getnames.aspx

1

Great answers here and probably answer your question better than what I'm going to mention...

Depending on how complicated your logic is based, you may consider using a strategy pattern like this:

Refactoring a Switch statement

or

The Strategy Template Pattern

Again, most likely more complicated than your solution asked, just throwing it out there...

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.