how to use indexOf proper to check if something is in the array

Question

I want to check in this guessing game if the color the user inputs is in the array list or not. but when u put a color that is in the list it keeps saying it isn't

var guess_input;
var target;
var colors = ["blue", "cyan", "gold", "gray", "green", "magenta", "orange",
  "red", "white", "yellow"
]
var finished = false;
var guesses = 0;
var pos;


function do_game() {
  var random_number = Math.floor(Math.random() * (colors.length - 1));
  target = colors[random_number];

  while (!finished) {
    guess_input = prompt("I am thinking of one of these colors: \n\n" + colors.join() +
      "\n\n What color am I thinking of?");
    guesses++;
    finished = check_guess();
  }
}

function check_guess() {
  if (colors.indexOf(guess_input) > -1) {
    alert(
      "This is not a color from the list ! Please pick a color from the list\n\n"
    );
    return false;
  }
}

Have you tried printing guess_input to the console to see what the value is? — ryuu9187, Oct 02 '15 at 17:36
Unless it's just a copy-and-paste error, looks like you have 2 issues: your close-bracket is on a new line under the last element of your comma-separated list of array values, and there is no semi-colon after it. Fix those two things and I think you'll be OK. What I suspect is happening is the array isn't being correctly defined in memory due to these syntax errors, thus no members of it can be found later. — Matt Campbell, Oct 02 '15 at 17:37
Why do you want to do this with indexOf? it's not exactly the most efficient way to check for set membership, unless the set is very small. (for other ways to do that, check out http://stackoverflow.com/questions/32782771/can-you-shorten-an-if-statement-where-a-certain-variable-could-be-multiple-thi ) — Touffy, Oct 02 '15 at 17:38
@SheraliTurdiyev you are right as === does a strict comparison and doesn't try to change types like ==. — Aakash, Oct 02 '15 at 17:44

score 3 · Answer 1 · answered Oct 02 '15 at 17:39

3

Your conditional is backwards:

function check_guess() {
  if (colors.indexOf(guess_input) === -1) { // -1 means not found! you have > -1 which means found!
    alert(
      "This is not a color from the list ! Please pick a color from the list\n\n"
    );
    return false;
  }
  else return true;
}

You also need to return true in some case to avoid an infinite loop.

answered Oct 02 '15 at 17:39

William B

1,411
8
10

I had the same answer - also the infinite loop comment is very important too! – Cameron Oct 02 '15 at 17:43
1

For brevity, you can omit the else keyword. – ryuu9187 Oct 02 '15 at 17:45
Good point jason. For maximum brevity we might write `function check_guess () { return ~colors.indexOf(guess_input) ? true : alert('This is not a color from the list! Please pick a color from the list\n\n'); }` – William B Oct 02 '15 at 17:48

score 2 · Answer 2 · answered Oct 02 '15 at 17:37

2

To check if the input is not in the list, you want == -1, not > -1.

  if (colors.indexOf(guess_input) == -1) {
    //the element was not found

answered Oct 02 '15 at 17:37

DLeh

23,806
16
84
128

how to use indexOf proper to check if something is in the array

2 Answers2