When to use FOR-CASE (Foreach/switch in C#)?

Question

I've found what seems to be the C# equivalent of a FOR-CASE structure in a proyect I'm working on:

foreach (string param in params.Split(';'))
{
    string[] parts = param.Split('=');
    string key = parts[0].Trim().ToLower();
    string value = parts[1].Trim();
    switch (key)
    {
        case "param1": this.param1 = value; break;
        case "param2": this.param2 = value; break;
        case "param3": this.param3 = value; break;
        case "param4": this.param4 = value; break;
        default: break;
    }
}

(Variable names changed to protect the guilty.)

How would you implement this code?

Answer 1

I don't think the code in your question is anything like the code you linked to....

The code in the question looks like something I might do if I wrote a command line tool.

Am I stupid for not seeing whats wrong with the code in the question?

An alternative is to use reflection to fill parameter value variables. I've done it that ways sometimes too.

BTW: I once wrote a program in a script language that had switch as the only flow control mechanism and no gosub/return. The code in my program was structured a bit like the one you linked to. A massive switch on a sort of instruction pointer variable that got reassigned at the end of every case and an almost infinite loop around the switch. It got the job done.

Answer 2

I see you that you already have multiple fields in your class that you use to hold the variables. In that case, what you are doing is fine.

Otherwise, you can have 1 HashTable (maybe add in the C# indexor as a twist) to hold all of them, and your loop will end up like this:

foreach (string param in params.Split(';'))
{
    string[] parts = param.Split('=');
    string key = parts[0].Trim().ToLower();
    string value = parts[1].Trim();
    MyHashTable[key] = value;
}

The problem with this approach is that you should only have 1 type of value. For example, if your param list can contain both string and int types, it makes the code messier, especially you need to perform error checking and validation and stuff.

I personally would stick with what you already have.

Answer 3

Not sure if I understand either but it sounds like you're complicating yourself. Don't reinvent the wheel, use BCL classes as much as you can, these classes are proven to work efficiently and save you lots of time. Sounds like you could implement it with some sort of Dictionary<,> along with, like Guge suggested, Reflection.

Answer 4

You could use reflection for this:

Type t = this.GetType();
foreach (string param in params.Split(';'))
{    
    string[] parts = param.Split('=');    
    string key = parts[0].Trim().ToLower();    
    string value = parts[1].Trim();    

    t.GetProperty(key).SetValue(this, value, null);
}

Answer 5

Or Regex:

string parms = "param1=1;param2=2;param3=3";
string[] parmArr = parms.Split(';');        

string parm1 = Regex.Replace(parmArr[0], "param1=", "");
string parm2 = Regex.Replace(parmArr[1], "param2=", "");
string parm3 = Regex.Replace(parmArr[2], "param3=", "");

Answer 6

For what it's worth, the WTF article was a WTF because its outer loop was completely useless, as noted in the article - it was just as easy, and more direct, just to set an index variable directly than to loop and test it.

Answer 7

I actually think the OP's code is fine. It's not perfect -- there might be simpler or cleaner ways to do it, but it effectively allows for readable mappings between member/property names and input-parameter names. It leaves your properties strongly typed (unlike the hashmap/dictionary solutions, unless your class has only one type for all its properties...) and gives you one fairly-obvious place to fix or add mappings.