CODEWITHSUNDEEP
javascriptif-statement
Sami
Sami

Reputation: 29

A function in ELSE statement is executed also with IF statement, how to prevent this?

I am trying to make this code executed only if there is a match between two values and I use if/else statement, and I put another function to change the background color to red in the "ELSE" statement but it is also executed even if the conditions are not met... When I click the first button "united states button" it executes very well, but when clicking any of the two other buttons it executes the the two statements i.e. write "Yes, Matched" and change the background color although it is in the ELSE statement.. how can I stop this??? Here is my code

const words = {
  "united states": "us",
  "united kingdoms": "uk",
  "united nations": "un",
};

function check(val) {

  for (word in words) {
    if (val == word) {
      document.getElementById('debug').innerHTML = "Yes, Matched";
      break;
    } else {
      document.getElementById('debug').innerHTML = "No Match";
      bgRed();

    }
  }
};

function bgRed() {
  for (i = 0; i < 3; i++) {
    document.getElementsByTagName('input')[i].style.background = 'red';
  }
}
* {
  text-align: center
}

input {
  margin: 30px;
}
<input type="button" value="united states" onclick="check(this.value)"><br>
<input type="button" value="united kingdoms" onclick="check(this.value)"><br>
<input type="button" value="united nations" onclick="check(this.value)">
<div id="debug"></div>

Upvotes: 0

Views: 696

Answers (3)

Erik McKelvey
Erik McKelvey

Reputation: 1627

In your for loop you are looping through all three of the keys in the dictionary. Since united states is the first key, the if statement will result to true on the first loop and the loop breaks.

When you are checking for the other values the if statement in the first iterations of the for loop will result to false, and the else statement will execute. After the first iterations the key will match and the if statement will result to true, but this is after the background has already been changed.

What I believe you want is to only execute the else statements after the loop has checked all the keys and there is no match, try the following code:

var found = false;
for (word in words) {
    if (val == word) {
        document.getElementById('debug').innerHTML = "Yes, Matched";
        found = true;
        break;
    }
}
if (found == false) {
    document.getElementById('debug').innerHTML = "No Match";
    bgRed();
}

Upvotes: 2

ADyson
ADyson

Reputation: 61794

The problem is that you take action inside the loop, where you loop through all the items in words object.

In the case of "united states", that's the first item in the list, so the code goes into the "yes matched" block, and then stops looping because of the break. But in the second and third cases, it has a chance to go into the else on the first loop, because it doesn't match. And there you set the background red. But you never change the background back again on the later loops.

A better solution would be to wait until the loop has finished before taking any visible action. Instead, just set a flag inside the loop which you later use to decide what action to take.

Demo:

const words = {
  "united states": "us",
  "united kingdoms": "uk",
  "united nations": "un",
};

function check(val) {
  var match = false;
  for (word in words) {
    if (val == word) {
      match = true;
      break;
    }
  }

  if (match == true) {
    document.getElementById('debug').innerHTML = "Yes, Matched";
  } else {
    document.getElementById('debug').innerHTML = "No Match";
    bgRed();
  }
};

function bgRed() {
  for (i = 0; i < 3; i++) {
    document.getElementsByTagName('input')[i].style.background = 'red';
  }
}
* {
  text-align: center
}

input {
  margin: 30px;
}
<input type="button" value="united states" onclick="check(this.value)"><br>
<input type="button" value="united kingdoms" onclick="check(this.value)"><br>
<input type="button" value="united nations" onclick="check(this.value)">
<div id="debug"></div>

Upvotes: 4

Thiago Romano
Thiago Romano

Reputation: 587

the reason is the for (word in words) loop, imagine you are trying to match every choice in the words list.

The reason only the United States does not turn red is that the loops break before it goes to the else statement.

What you want is probably a 

var found = false;
for (word in words) {
    if (val == word) {
      document.getElementById('debug').innerHTML = "Yes, Matched";
      found = true;
      break;
    }
}
if found == true {
 // do something
} else {
 // do something else
}

Upvotes: 6

Related Questions