JavaScript prints the entire array instead of single element

Question

var input = document.querySelector("input");
var button = document.querySelector("button");
var inventory = ["jacket","pants","map"];
var actionsIKnow = ["north","east","west","south"];
var messages = ["A group of dangerous minions. Turn back, you do not have a sword",
 "You have gone too deep into the forest. Want to go further?",
 "A giant ruby crystal",
 "A silent lake; you see your reflection in the water",
 "Clear patch of land",
 "A sleeping dragon. Bettwe not wake it up",
 "A solitary cottage. Faint music can be heard from the inside",
 "Looks like this path leads to the cottage of the flute maker",
 "A lot of tombstones, looks like an old graveyard"
];
var userInput;
var startingPos = 4;
button.addEventListener("click",takeMeThere,false);
function takeMeThere(){
 userInput = input.value;
 userInput = userInput.toLowerCase();
 if(userInput!=null){
 validateInput();
 }
}
function validateInput(){
 for(var i=0;i<actionsIKnow.length;i++){
 if(userInput.indexOf(actionsIKnow[i]!=-1)){
 move(actionsIKnow[i]);
 }
 }
}
function move(where){
 console.log(where);
}

I am making a text based exploring game where the user can select where to go. Where the user wants to go depends on what was entered in the textfield. This data is then passed to move(where) where I console.log(where). Instead of printing north, east, etc it prints the whole of actionsIKnow array. Why ?

• nnnnnn: Not the problem, but note that the test if(userInput!=null){ will never be false.
• An SO User: @nnnnnn why ? How do I check if user enter empty ?
• nnnnnn: The .value attribute returns a string, and the string method .toLowerCase() also returns a string. To test that the field is not empty compare to an empty string: if (userInput!=""){. You may also like to first trim any white space: userInput = userInput.replace(/\s/g,"");, in case the user enters nothing but spaces...

Answer 1

Simple mistake. Change

if(userInput.indexOf(actionsIKnow[i]!=-1)){

to this:

if (userInput.indexOf(actionsIKnow[i]) !== -1 ) {

http://jsfiddle.net/bz8k5/

Edit. As per discussion in comments. To be able to validate input in different format like east, move east, but disallow easter you may also want to make use of more sophisticated validation rules. Maybe using regular expressions:

if (userInput.match(new RegExp("\\b" + actionsIKnow[i] + "\\b"))) {
 move(actionsIKnow[i]);
}

http://jsfiddle.net/bz8k5/2/

• nnnnnn: +1. You just beat me - guess I shouldn't have "wasted" time creating a fiddle: jsfiddle.net/HuyaT
• dfsq: @nnnnnn ))) I created a fiddle after I answered.
• Michael Geary: Enter "feasting" into the input box and click Go. It prints "east". Should it?
• nnnnnn: Further to what Michael said, the user might enter "North East", and then it would print both "north" and "east" - is that right? (More a question for the OP than for dfsq...)
• dfsq: @MichaelGeary Right, this is how OP searches the value in the array. Of course it still needs some work. For example I would use indexof method of the array. jsfiddle.net/bz8k5/1 But this is another question.

Answer 2

Look closely at this line of code:

if(userInput.indexOf(actionsIKnow[i]!=-1)){

Let's add spaces to make it easier to see:

if( userInput.indexOf( actionsIKnow[i]!=-1 ) ) {

See it yet? Let's turn the spaces into newlines and indent:

if(
 userInput.indexOf(
 actionsIKnow[i] != -1
 )
) {

Now do you see the problem? :-)

Anyway, what you probably really meant here is simply:

if( userInput == actionsIKnow[i] ) {

Or, as @nnnnnn suggested, you can use .indexOf() instead of your own loop, like this:

function validateInput(){
 if( actionsIKnow.indexOf(userInput) >= 0 ) {
 move( userInput );
 }
}

But beware of one problem: .indexOf() on an array is not available in IE8 and earlier versions of IE. If you need to support IE8, this won't work.

So here is my favorite way to do it: Use an object lookup instead of an array.

Change actionsIKnow to:

var actionsIKnow = { north:1, east:1, west:1, south:1 };

and then change validateInput() to:

function validateInput(){
 if( userInput in actionsIKnow ) {
 move( userInput );
 }
}

This works in all browsers, and if there were a large number of items in actionsIKnow, this object lookup would be much faster than .indexOf(). For a small number of items the performance wouldn't matter, but I think the in operator makes for the nicest reading code.

Here's a jsperf that compares performance between .indexOf() and in for a large number of strings.

• nnnnnn: In your updated function that uses the in operator, what is i? Should be move(userInput)...
• Michael Geary: Oops, sorry about the typo, just fixed it. The function doesn't use i at all any more. It doesn't need to loop.
• nnnnnn: Cool. Even with an array the OP could skip the loop and just use the array .indexOf() method.
• Michael Geary: Very true. Of course .indexOf() does loop, it just does it for you. If you were checking against a very large dictionary of words, putting them in an object and testing with the in operator should be faster than .indexOf(). But for a small array performance would be fine either way.
• ErikE: Michael, how sure are you that the object is faster? Doesn't it have to loop just the same, under the covers?