Javascript if statement with multiple elseifs

Question

I have the below if statement in use to compare 2 rows in the same column in a table. I use the same block of code multiple times to perform the same calculation on different columns.

Basically, I want to have an arrow show both the movement in value between one row and the next, as well as the threshold the value falls into. So, for example, in my code below anything above 15 should be a red arrow and if the previous value was lower then the arrow should point up. I render the arrows using images called from another location later in my code, so the if statement just gives me the string for the appropriate arrow.

As I said above, I have the same block of code numerous times. The strange thing is, the output works for some variables and does't for others; even though it's the exact same code with the variable names replaced! It gives me the wrong are SOMETIMES for no apparent reason.

Do I have too many conditions? Is there a more efficient way to do what I'm trying to do?

Any assistance would be greatly appreciated!

Thanks, Karl

var Arrow = " ";
if(KPI[0] > KPI[1] && KPI[0] >= 15)
{
    Arrow = "redarrowup.png";
}
else if(KPI[0] < KPI[1] && KPI[0] >= 15)
{
    Arrow = "redarrowdown.png";
}
else if(KPI[0] = KPI[1] && KPI[0] >= 15)
{
    Arrow = "redarrowflat.png";
}
else if(KPI[0] > KPI[1] && KPI[0] >= 10 && KPI[0] < 15)
{
    Arrow = "yellowarrowup.png";
}
else if(KPI[0] < KPI[1] && KPI[0] >= 10 && KPI[0] < 15)
{
    Arrow = "yellowarrowdown.png";
}
else if(KPI[0] = KPI[1] && KPI[0] >= 10 && KPI[0] < 15)
{
    Arrow = "yellowarrowflat.png";
}
else if(KPI[0] > KPI[1] && KPI[0] < 10)
{   
    Arrow = "greenarrowup.png";
}
else if(KPI[0] < KPI[1] && KPI[0] < 10)
{   
    Arrow = "greenarrowdown.png";
}
else
{
    Arrow = "greenarrowflat.png";
}

Answer 1

At first glance I'd say that you have an "=" where you need an "==" (or even "==="). For example

if(KPI[0] = KPI[1] && KPI[0] >= 15)

should probably be

if(KPI[0] == KPI[1] && KPI[0] >= 15)

(or even === as I said). And so forth. "=" is an assignment operator, it sets the value of KPI[0] to the value of KPI[1] and then returns that value, so you are essentially doing the following:

KPI[0] = KPI[1];
if(KPI[1] && KPI[0] >= 15)

which I doubt is the intention. That said, as several others have already said there are better ways to structure the code.

Answer 2

I recommend expressing the different concepts separately and then combining them at the end. Like this:

var arrow;
var movement = "flat";
var band = "green";

if (KPI[0] > KPI[1]) {
    movement = "up";
} else if (KPI[0] < KPI[1]) {
    movement = "down";
}

if (KPI[0] >= 15) {
    band = "red";
} else if (KPI[0] >= 10) {
    band = "yellow";
}

arrow = band + "arrow" + movement + ".png";

You could also defer each of these elements to a function, so your final line would read:

arrow = getBand(KPI[0], KPI[1]) + "arrow" + getMovement(KPI[0], KPI[1]) + ".png";

This would make it easier to change the bands in isolation of this other logic.

Answer 3

I would say:

function direction(val1, val2){
  return val1 < val2 ? "down" : val1 == val2 ? "flat" : "up";
}

function color(val){
  return val >= 15 ? "red" : val >= 10 ? "yellow" : "green";
}

var arrow = color(KPI[0]) +"arrow"+ direction(KPI[0], KPI[1]) +".png";

I always tell people not to copy and paste code if you don't understand it. You might be unfamiliar with the ?: bit. It is a ternary operator (inline if).

If you need multiple arrows I would suggest to add a function for generating an arrow too:

function arrow(val1, val2){
  return color(val1) +"arrow"+ direction(val1, val2) +".png";
}

var kpiArrow = arrow(KPI[0], KPI[1]);

Answer 4

I don't think the problem is in the number of if's. But you can structure your code better:

var direction, color;

// set direction

if(KPI[0] > KPI[1])
{
  direction = "up";
}
else if(KPI[0] < KPI[1])
{
  direction = "down";
}
else if(KPI[0] == KPI[1]) // there was an error in this condition, by the way
{
  direction = "flat";
}

// set color

if(KPI[0] >= 15)
{
  color = "red";
}
else if(KPI[0] >= 10 && KPI[0] < 15)
{
  color = "yellow";
}
else if(KPI[0] < 10)
{
  color = "green";
}

var Arrow = color + "arrow" + direction + ".png";

Answer 5

How about this:

var arrow = "";

if (KPI[0] >= 15) {
   arrow = "redarrow";
}
else if (KPI[0] >= 10) {
   arrow = "yellowarrow";
}
else if (KPI[0] < 10) {
   arrow = "greenarrow";
}

if (KPI[0] < KPI[1]) {
   arrow += "down.png";
}
else if (KPI[0] > KPI[1]) {
   arrow += "up.png";
}
else if (KPI[0] == KPI[1]) {
   arrow += "flat.png";
}