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.

Philipp M
  • 1,877
  • 7
  • 27
  • 38
  • 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 01 '10 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 01 '10 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 01 '10 at 19:25

9 Answers9

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
    }
}
Kirk Woll
  • 76,112
  • 22
  • 180
  • 195
  • 1
    i recommend to pre-calculate `Enum.GetValues(typeof(Commands))` and store somewhere. it is not cheap operation. – Andrey Oct 01 '10 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 01 '10 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 01 '10 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 01 '10 at 15:21
  • @Brian agree. i should have sad: if it is bottleneck then precalc it. – Andrey Oct 01 '10 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).

Ani
  • 111,048
  • 26
  • 262
  • 307
  • 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 01 '10 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);
    }
}
Andrey
  • 59,039
  • 12
  • 119
  • 163
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();
}
Brian Gideon
  • 47,849
  • 13
  • 107
  • 150
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
        }
    }
}
ThatBlairGuy
  • 2,426
  • 1
  • 19
  • 33
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.

Carvellis
  • 3,992
  • 2
  • 34
  • 66
  • +1 Enums are certainly the way to go for collections of like preset values. – BoltClock Oct 01 '10 at 14:20
  • @Heinzi: As Kirk indicates, that's what `Enum.GetValues` is for. – Brian Oct 01 '10 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 01 '10 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 01 '10 at 14:58
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.

Steve Townsend
  • 53,498
  • 9
  • 91
  • 140
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

SKG
  • 1,432
  • 2
  • 13
  • 23
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...

Bryce Fischer
  • 5,336
  • 9
  • 30
  • 36