5

I'm dealing with quite a lot of cases in my switch() statements and was wondering if there's any possible way I could shorten these. They take up a lot of space in my code and it's harder to navigate when there are 3-4 big chunks of these statements. Here's an example:

...important lines of code...

void foo(string bar, bool blam) {

     int v1 = stoi(bar);

     switch (v1) {
         case(11):
             if(blam) {
                exArr[1] = "A";
             } else {
                exArr[1] = "B";
             }
             break;

         case(12):
             if(blam) {
                 exArr[3] = "A";
             } else {
                 exArr[3] = "B";
             }
             break;

         ...many more cases...

         default:
             printElement();
             break;
}
...even more important code, which is dependent on the hard code above and hard to navigate...

I think you see the problem. Do you guys have any suggestions? Thanks in advance.

IMPORTANT EDIT:

Only the first 12 iterations change the characters of exArr. After that, it changes to another (existing) array, like ndArr, which takes another 12 iterations. This goes on for 4 arrays, so about 48 case statements.

Lightness Races in Orbit
  • 378,754
  • 76
  • 643
  • 1,055
splashrt
  • 81
  • 4
  • 8
    Is there a relationship between the value of `v1` and the index in `exArr` which you can write as a mathematical formula? – eesiraed Sep 21 '19 at 18:58
  • 1
    Does `blam` always distinguish wanting `"A"` and `"B"`? – Caleth Sep 21 '19 at 19:03
  • If the code has the same pattern of the snippet then just an array of integers or `std::vector`. Index is the value of v1 and the value is the index of the item in exArr. Like `exArr[map[v1]] = blam ? "A" : "B" `. No need for a switch. – Adriano Repetti Sep 21 '19 at 19:04
  • Something like `extraArr[1] = blam ? "A" :"B";` would make this at least a little more compact. –  Sep 21 '19 at 19:04
  • If in every `case` statement you're asking about `blam`, then probably you want something like `if (blam) { switch()... } else { switch()... }`, which means you have two shorter `switch`es and just one if. – Mirko Sep 21 '19 at 19:26
  • @AlexanderZhang Let `v1 = 11`. For any `v1` with the last digit of 1, the index is `exArr[1]`. Now let `v1 = 12`. The index will now be `exArr[3]`. For `v1 = 13` it will be `exArr[5]` and so on, which is probably not impossible to compile into a mathematical formula. The hard part comes in when the first digit changes. Let `v1 = 21`. We've learned that it will be `exArr[1]`, but the array has to be changed: it now goes into `ndArr[]`, another existing array. And that goes on - I've stated it in my edit, sorry for not clarifying earlier. Any propositions for this particular problem? – splashrt Sep 21 '19 at 19:26
  • @Caleth yes, it always does. – splashrt Sep 21 '19 at 19:27
  • BTW, parenthesis are not required in a `case` statement. – Thomas Matthews Sep 21 '19 at 19:42
  • Use a spread sheet. One column is for the `case` values, another column for the array name, another for the array index and the last for the value to assign to the array element. Enter all your data into the spreadsheet, one row per `case`. Look for any relationships between `case` value and the other columns. Maybe even apply some `Database Normal Forms`. You did not supply enough cases, so I can't deduce any relationships for you. – Thomas Matthews Sep 21 '19 at 19:46
  • @splashrt, use `if`/`then`/`else` statements that check in which range`v1` lies, and then assign the appropriate array element based on the relation of `v1` for that particular range and the array and index. I would not hardcode such relations through switch statements, it does not scale. – Ton van den Heuvel Sep 21 '19 at 19:47
  • Create a temporary variable that contains `0` for when `blam` is `true` and `1` when `blam` is false. Define variable before the `switch`. Your assignment statements will look like: `exArr[1] = 'A' + blame_value;`. – Thomas Matthews Sep 21 '19 at 19:49
  • The key to simplifying this sort of code is looking for similarities and differences, which unfortunately is difficult to do without seeing the whole `switch`. So I'd leave that as a task for you. What is the same in each of these options? What is different? What patterns are there in the differences? Please summarize the content/purpose of the full `switch` to move beyond good answers that just don't go far enough. – JaMiT Sep 21 '19 at 20:53

4 Answers4

4

Well first you can remove the curly braces on if else statements, since they are one liners, inside the case blocks like this

...
case(12):
    if(blam)
        exArr[3] = "A";
    else
        exArr[3] = "B";
    break;
...

and you can take it a step further and use Tenary Operators which would look like this

...
case(12):
    exArr[3] = (blam) ?  "A" : "B";
    break;
...
1201ProgramAlarm
  • 32,384
  • 7
  • 42
  • 56
sassy_rog
  • 1,077
  • 12
  • 30
  • I added the suggestion before he clarified that he had to modify different arrays. In that case as you say you can use nested switch statements or use case(12): return exArr[1] = blam ? "A" : "B"; – Wolfgang Brehm Sep 21 '19 at 20:03
4

As @Alexander Zhang mentioned, if you have a particular algorithm you can use, the simplest solution to your problem would be similar to what @Ton van den Heuvel proposes.

If not, there is also the alternative of using a lookup table (referenced from here) if you have particular values that match up.

Eg. 

#include <map>

.../

map<int,int> mapV1toIndex = {
    {11, 1}, 
    {12, 3},
    .../
};

void foo(string bar, bool blam) {
    int v1 = stoi(bar);
    exArr[mapV1toIndex[v1]] = (blam) ?  "A" : "B";
}

Also, if you wanted to use different string arrays each time, you could pass in the string array into foo like so to reuse the foo function: 

void foo(string *pStrArray, string bar, bool blam) {
    int v1 = stoi(bar);
    pStrArray[mapV1toIndex[v1]] = (blam) ?  "A" : "B";
}

Edit: It is preferable to use std::map instead of a struct. Edited the code to use map, following this reference.

Enthus3d
  • 1,727
  • 12
  • 26
  • I need to test my struct a bit to make sure it works, but this would be the general form of the answer, and you could potentially make it into 4 ternary statements. – Enthus3d Sep 21 '19 at 19:22
  • Realized the struct container is not as good for use as simply using std::map. Edited my answer to use map instead. – Enthus3d Sep 21 '19 at 20:01
  • Added in a tidbit about reusing foo for multiple arrays. – Enthus3d Sep 21 '19 at 21:03
2

Assuming the relation between v1 and the subsequent cases continues in the same way, a switch is not needed:

const int v1{stoi(bar)};
if (11 <= v1 && v1 <= ...)
  exArr[1 + (v1 - 11) * 2] = blam ? "A" : "B";
else
  printElement();
Ton van den Heuvel
  • 10,157
  • 6
  • 43
  • 82
  • 3
    While this is not the right solution as OP apparently really needs a switch statement it is still a clever idea and I find the downvotes to be quite undeserved... – Wolfgang Brehm Sep 21 '19 at 19:31
0

You should ideally try to solve this issue by using polymorphism. I found this on stackoverflow, Ways to eliminate switch in code. The example isn't directly implemented in C++ but it will give you an idea. By using polymorphism, your code will be lot more readable and easily extensible in the future.

  • This is only really applicable to object oriented code. I think these kinds of answers which lack implementation should be relegated to the comments instead. – Enthus3d Sep 21 '19 at 22:13