views:

266

answers:

9

Im 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. I'm happy if anyone can point me to some article or even more if there is a short answer to my problem. Thanks in advance :)

+2  A: 

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.

Jappie
+1 Enums are certainly the way to go for collections of like preset values.
BoltClock
`DifferentMethod` does not work any more after the change...
Heinzi
You can't cast `args[0]` directly to `Command`. Use `Parse` or `TryParse` instead.
digEmAll
@Heinzi: As Kirk indicates, that's what `Enum.GetValues` is for.
Brian
@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.
Jappie
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
+4  A: 

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);
    }
}
Andrey
+2  A: 

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.

Steve Townsend
+14  A: 

Convert Commands into an enum:

enum Commands { ComandOne, CommandTwo, CommandThree, CommandFour }

Switch statement should look like:

static void Main(string[] args)
{
    Command = 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
    }
}
Kirk Woll
+1 This looks to be the simplest way to get where the OP wants to go.
Chris Lively
i recommend to pre-calculate `Enum.GetValues(typeof(Commands))` and store somewhere. it is not cheap operation.
Andrey
+1 Stable and easy to read.
RedFilter
@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
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
@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
@Brian agree. i should have sad: if it is bottleneck then precalc it.
Andrey
+5  A: 

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).

Ani
+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
A: 

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

SKG
+1  A: 

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...

Bryce Fischer
+3  A: 

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();
}
Brian Gideon
+1, I didn't know about ICommand before. Thanks!
FrustratedWithFormsDesigner
@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.
Brian Gideon
@FrustratedWithFormsDesigner: I edited my answer to make it more clear.
Brian Gideon
@BrianGideon: 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.
FrustratedWithFormsDesigner
+1 for best in show
Biff MaGriff
+1  A: 

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.

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 = command1;
        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
        }
    }
}
ThatBlairGuy
This is very simple and exactly what I need. Thank you all for you great answers.
Philipp Maretzky