Search array for string - return -1

I understand that you have already fixed the error, but I urge you to consider a different approach to the problem. These two principles not only solve the problem in a cleaner way, but also give you a new way to think about similar problems in the future:

• Never use parallel arrays. Use only one array of objects instead.
• In your main loop that adds elements, put the main loop element in a function.

If you follow these two ideas, you will reap several benefits. The code becomes much simpler, easier to maintain, and you don't need to do any array lookups at all!

Each store item is packaged as one object in an array, for example:

var shopItems = [
    { name: 'Sword', cost: 20 },
    { name: 'Shield', cost: 25 }
];

      

        
        
        
      

    

So, if you have a link to a store item as a whole, say in a variable under the name shopItem

, then you will automatically get all your properties: shopItem.name

and shopItem.cost

. It also makes it easy to add more bits of data to a store item, for example.

var shopItems = [
    { name: 'Sword', cost: 20, dangerous: true },
    { name: 'Shield', cost: 25, dangerous: false }
];

      

        
        
        
      

    

and will now shopItem.dangerous

provide you with the corresponding value. All without searching for arrays.

Casting the body of the main loop into a function adds an additional benefit: inside the function, its parameters and local variables are saved every time you call the function (this is called a closure). So now you don't even need to fetch the value of the list item and search for it - you already have the corresponding shopItem

one available in your code.

Putting this together, the code might look like this:

var shopItems = [
    { name: 'Sword', cost: 20, dangerous: true },
    { name: 'Shield', cost: 25, dangerous: false }
];

var list = document.getElementById( 'list' );

for( var i = 0;  i < shopItems.length;  ++i ) {
    appendShopItem( shopItems[i] );
}

// Alternatively, you could use .forEach() instead of the for loop.
// This will work in all browsers except very old versions of IE:
// shopItems.forEach( appendShopItem );

function appendShopItem( shopItem ) {
    // Create the list item:
    var item = document.createElement( 'li' );

    // Set its contents:
    item.appendChild( document.createTextNode( 
        shopItem.name + ' - ' + shopItem.cost + ' Gold'
    ) );

    // Add it to the list:
    list.appendChild( item );

    var label = document.createElement( 'label' );
    var radio = document.createElement( 'input' );
    var text = document.createTextNode( shopItem.name );
    radio.type = 'radio';
    radio.name = 'shop';
    radio.value = shopItem.name;
    radio.onclick = function () {
        addValue( shopItem );
    };

    label.appendChild( radio );
    label.appendChild( text );
    document.body.appendChild( label ); 
}

function addValue( shopItem ) {
    console.log( shopItem );
    alert(
        shopItem.name +
        ' costs ' + shopItem.cost + ' and is ' +
        ( shopItem.dangerous ? 'dangerous' : 'not dangerous' )
    );
}

      

        
        
        
      

    

New violin (with the tip of the hat to Jamiec for the original violin)

As you can see, this makes the code much clearer. If you have shopItem

, you automatically have name

, cost

and any other property you want to add. And most importantly, you never need to keep track of your values ​​in the same order in two, three, or even different arrays.