0

I currently have a piece of code that looks like this:

switch (objectname)
{
    case "objectbla":
        DoSomething(1, objectName, someOtherVar);
         break;
    case "objectblabla":
        DoSomething(2, objectName, someOtherVar);
        break;
    case "objectetc":
        DoSomething(3, objectName, someOtherVar);
        break;
    case "objectanother":
        DoSomething(4, objectName, someOtherVar);
        break;
    case "objectobj":
        DoSomething(5, objectName, someOtherVar);
        break;
    default:
        break;
}

Now, seeing how repetitive this switch is with only the first parameter counting up once, I'm sure this could be written more efficiently. I'm not sure, though. What would be a better way to write this?

poke
  • 369,085
  • 72
  • 557
  • 602
  • 2
    Well if `objectname` was an `enum`, the enum's value could be passed into `DoSomething((int)yourEnumValue, objectName, someOtherVar);` – DiskJunky Oct 04 '17 at 13:24
  • 3
    put all your strings to a dictionary `Dictionary` and make a single call `DoSomething(dict[objectname], objectName, someOtherVar)` – L.B Oct 04 '17 at 13:24
  • 2
    or `Array.IndexOf(stringArray, objectname) + 1` – Alex K. Oct 04 '17 at 13:27
  • You can also `Enum.TryParse` to get `3` from an underlying `AnEnum.ObjectEtc = 3` – Alex K. Oct 04 '17 at 13:32

4 Answers4

5

If the first parameter is the only thing that is different depending on objectname, you should consider using a dictionary for that:

// you only have to set this up once
var lookup = new Dictionary<string, int>()
{
    ["objectbla"] = 1,
    ["objectblabla"] = 2,
    ["objectetc"] = 3,
    ["objectanother"] = 4,
    ["objectobj"] = 5,
};


// actual logic
if (lookup.TryGetValue(objectname, out var objectId))
{
    DoSomething(objectId, objectName, someOtherVar);
}
else
{
    // objectname is not in the lookup dictionary
}

This is the general idea. Depending on how your lookup looks you could also choose different solutions, but the dictionary is the most verbose but also most flexible way to do it here.

poke
  • 369,085
  • 72
  • 557
  • 602
1

If it has to be an switch, How about:

int  aNumber;
switch (objectname)
{
    case "objectblabla":
        aNumber = 1
        break;
    case "objectetc":
        aNumber = 2
        break;
    case "objectanother":
        aNumber = 3
        break;
    case "objectobj":
        aNumber = 4
        break;
    default:
        break;
}

DoSomething(aNumber, objectName, someOtherVar);

And if not:

string[] ListOfObjectNames = { "objectblabla", "objectetc", "objectanother" };
DoSomething(Array.IndexOf(ListOfObjectNames, objectname), objectName, someOtherVar);
Salah Akbari
  • 39,330
  • 10
  • 79
  • 109
1

You are correct that there is a better way. If you create a static Dictionary as lookup table, then you use that to get your magic numbers.

static Dictionary<string, int> lookup= new Dictionary<string, int>()
{
    { "objectbla",1},
    {"objectblabla", 2},
  etc.
};

Then the body of your function becomes:

DoSomething(lookup[objectname], objectName, someOtherVar);

You should also add code to take account of the possibility of an invalid key being used, otherwise, it will throw an exception as it stands.

Dragonthoughts
  • 2,180
  • 8
  • 25
  • 28
1

I'd go with the enum method.

enum objects
{
    objectbla = 1,
    objectblabla,
    objectetc,
    objectanother,
    objectobj
};

DoSomething((int)Enum.Parse(typeof(objects), objectName), objectName, someOtherVar);
spodger
  • 1,668
  • 1
  • 12
  • 16