How can I simplify my long if else statement

Question

I hope someone could help me to simplify this expression. As shown below there are 2 values that will be checked. Acoording to both of these values, I have to update an image. I tried some stuff but everything leads back to this long if else statement I am not sure whether this has been asked before since I am not sure what to search for. If you could provide me some known previous question would be helpful too. I have value x and y which represent the position of a gps receiver from the center of a car. And depending on this position, I have to update the position of the gps in the image shown to the user.

    if (val > 0)
    {
       if (secondVal > 0)
       {
          SetIcon("right_front.png")
       }
       else if (secondVal< 0)
       {
          SetIcon("right_back.png")
       }
       else
       {
          SetIcon("right_center.png")
       }
    }
    else if (val < 0)
    {
       if (secondVal > 0)
       {
          SetIcon("left_front.png")
       }
       else if (secondVal < 0)
       {
          SetIcon("left_back.png")
       }
       else
       {
          SetIcon("left_center.png")
       }
    }
    else
    {
       if (secondVal > 0)
       {
          SetIcon("center_front.png")
       }
       else if (secondVal < 0)
       {
          SetIcon("center_back.png")
       }
       else
       {
          SetIcon("center_center.png")
       }
    }

Answer 1

Wel, let's face it: you need to cover the following cases:

val Secondval
  0         0
  0        >0
  0        <0
 >0         0
 >0        >0
 >0        <0
 <0         0
 <0        >0
 <0        <0

Those are nine cases, and your if-else loop contains nine cases, so I don't think it's possible to simplify this.

Obviously, if there are some doubles in the actions that need to be taken, the whole story can change:

val Secondval  action to take
  0         0        action_1
  0        >0        action_1
  0        <0        action_1
 >0         0        action_2
 >0        >0        action_2
 >0        <0        action_2
 <0         0        action_2
 <0        >0        action_2
 <0        <0        action_2

Here, you might simplify to:

if (val == 0) {
  action_1;
} else {
  action_2;
}

So, if you do have repeating actions, it might be interesting to make a table like the one I showed and see what you can do.

Answer 2

You could do something like this for your specific case

    std::string icon;

    if (val > 0){
        icon = "right";
    }
    else if (val < 0){
        icon = "left";
    }
    else{
        icon = "center";
    }

    if (secondVal > 0)
    {
        icon += "_front.png";
    }
    else if (secondVal< 0)
    {
        icon += "_back.png";
    }
    else
    {
        icon += "_center.png";
    }

    SetIcon(icon);

Answer 3

Here's one approach:

std::string direction = val ? val < 0 ? "left" : "right" : "center";
std::string orientation = secondVal ? secondVal < 0 ? "back" : "front" : "center";

setIcon(direction + "_" + orientation + ".png");

Answer 4

Of course, this code can be simplified, because all your 9 actions are more or less the same. Base your solution on the signum function (which returns -1 for numbers less than 0, 0 for 0, and 1 for numbers greater than 0). Here is a signum implementation from another question here:

template <typename T> int sgn(T val) {
    return (T(0) < val) - (val < T(0));
}

Now the idea is to convert your numbers val and secondVal to an integer from 0 to 8 and then use that as index into an array.

int idx = (sgn(val)+1)*3+(sgn(secondVal)+1);
SetIcon(icons[idx]);

The first part converts val into integers 0, 3, and 6 based on whether it is negative or not. Then we add the secondVal as 0, 1, and 2, again, based on whether it is negative or not. You just have to create an array with the correct ordering of the values.

char *icons[] = { "left_back.png", "left_center.png", "left_front.png",
                  ... };

Answer 5

Having three versions of essentially the same code suggests that you need to write a function:

void select_icon(int value, const char* gt_value, const char* lt_value, const char* eq_value) {
    if (value > 0)
        SetIcon(gt_value);
    else if (value < 0)
        SetIcon(lt_value);
    else
        SetIcon(eq_value);
}

and then just call it:

if (val > 0)
    select_icon(secondValue, "right_front.png", "right_back.png", "right_center.png");
else if (val < 0)
    select_icon(secondValue, "left_front.png", "left_back.png", "left_center.png");
else
    select_icon(secondValue, "center_front.png", "center_back.png", "center_center.png");

This particular problem has more commonalities that can be exploited, as other answers have suggested: build the name on the fly, or use a two-dimensional array indexed by signum. I mention refactoring because that should probably be your first instinct when you see redundant code. My inclination is to write a function whenever I have more than one level of logic nesting. Sometimes it's easier to keep the code all in a single function, but most of the time refactoring makes things much smaller and clearer.