0

please have a look on my if condition. I am just refreshing my javascript and I am wondering, how I could check, if the inserted variables, are the ones I want to be used.

Obviously the "game" should only take rock, paper or scissors.

Now the if condition says, if (choice 1 and choice 2 equal rock or scissors or paper) { do this; } else {do something else}

But apparently it is not working the way I want.

var choice1 = prompt("Player 1: Rock, scissors or paper?");
var choice2 = prompt("Player 2: Rock, scissors or paper?");

compare(choice1, choice2);

function compare(choice1, choice2) {

if(choice1 && choice2 === "rock" || "paper" || scissors) {
    alert("You pass");
} else {
    alert("Something went wrong");
}

Could anyone give me a short explanation, why the if condition passes every value it gets? It never shows the mssage "Something went wrong".

  • possible duplicate of [javascript, creating a rock, paper, scissors game](http://stackoverflow.com/questions/17760831/javascript-creating-a-rock-paper-scissors-game) – Aadit M Shah Nov 08 '13 at 02:20

7 Answers7

2

I believe it shoud look like:

if ((choice1=="rock" || choice1=="paper" || choice1=="scissors") && 
    (choice2=="rock" || choice2=="paper" || choice2=="scissors")){...
Chong Lip Phang
  • 8,755
  • 5
  • 65
  • 100
  • if((choice1 === "rock" || "paper" || "scissors") && (choice2 === "rock" || "paper" || "scissors")) { document.write("Form validated

    The Game can continue.") } else { document.write("Wrong. Access denied.") } That did not work either... strange...
    – bjanka muhametaj Nov 08 '13 at 02:12
  • It certainly doesn't work, because "paper" and "scissors" evaluate to true for the expression. It is like writing ((choice1==="rock")||("paper")||("scissors)).You can't compare several literals with a variable like that. – Chong Lip Phang Nov 08 '13 at 02:16
  • Ok, now I understood. Will check in my browser and give feedback here. – bjanka muhametaj Nov 08 '13 at 02:17
1

The issue is your condition will always evaluate to true since the || "paper" condition is going to return a truth-y value. That alone makes the entire condition true, since the values are OR'd, so it always passes. You're checking for choice1 && choice2 === "rock", which isn't really written correctly. As for || scissors that will be false since scissors is undefined here.

Consider taking this approach instead:

var validOptions = ["rock", "paper", "scissors"];
if(validOptions.indexOf(choice1) > -1 && validOptions.indexOf(choice2) > -1) {
    // now compare choice1 and choice2 to determine the winner
} else {
    alert("Something went wrong");
}

Note that the solution shown doesn't trim the user input or account of case sensitivity. To address that you could use a regex with the ignore-case flag:

var re = /\b(?:rock|paper|scissors)\b/i;
if (re.test(choice1) && re.test(choice2)) {
    // ...
}
Ahmad Mageed
  • 94,561
  • 19
  • 163
  • 174
  • This is currently beyond my current Javascript comprehension. But I am working on it. Thank you for your input. By the way, I put the scissors in double quote, but the result didn't change either. – bjanka muhametaj Nov 08 '13 at 02:16
  • @bjankamuhametaj no problem. The first part of this is the simpler solution. Regex is a little more advanced. Double or single quotes don't make a difference. It's a personal preference, and either represent a string. The first solution places all the valid options into an array named `validOptions`. Next, we check if the `choice1` value exists amongst the valid options. This is done by using the array's `indexOf` method, and passing `choice1`. `indexOf` returns `-1` if the item doesn't exist. If it does the `> -1` condition passes. The same goes for `choice2`. – Ahmad Mageed Nov 08 '13 at 02:21
1

There are lots of indexOf answers with arrays, but you can also use an object for valid resopnses:

var validResponses = {rock:'', paper:'', scissors:''};

if (choice1 in validResponses && choice2 in validResponses) {
  // all good
}

of if you want be safe:

if (validResponses.hasOwnProperty(choice1) && validResponses.hasOwnProperty(choice2)) {
  ...
}
RobG
  • 142,382
  • 31
  • 172
  • 209
  • This can be combined with sub-objects holding 2 integers each (one for wins against, one for lose against) and then bitwise checks can do the win logic **very** quickly. – Paul S. Nov 08 '13 at 11:17
0
if(choice1 && choice2 === "rock" || "paper" || scissors) {

Could anyone give me a short explanation, why the if condition passes every value it gets?

Because it will be evaluated as

if ((choice && (choice2 === "rock")) || "paper" || scissors )

which is probably not what you wanted. The non-empty string "paper" is a truthy value, which means it will always fulfill the condition (and due to short-circuit evaluation, the reference error scissors will not be looked at).

See Check variable equality against a list of values (and its many linked duplicates) for how to do it correctly.

Community
  • 1
  • 1
Bergi
  • 630,263
  • 148
  • 957
  • 1,375
0
function compare(choice1, choice2){
  var identical = choice1 === choice2;
  var options = ["rock", "paper", "scissors"];

  if (identical && (options.indexOf(choice1) > -1))
    alert("Pass");
  else
    alert("Not alike");

}

In this code, identical will be either true or false, and if it is true, you can check if it matches any of the values inside the options array.

Jason
  • 11,263
  • 21
  • 87
  • 181
0

Your code is being evaluated like this:

if (
    choice1 // Non-empty strings always evaluate to TRUE
    &&
    choice2 === "rock"
    // The above are evaluated together
    // If both are not TRUE, FALSE is the result

    ||
    "paper" // Non-empty string. TRUE
    ||
    "scissors" // Non-empty string. TRUE
    )

This pseudo code might help:

if (value of AND block
    ||
    true
    ||
    true)

Since OR has a true available every time, the if block will run every time.

Tyler Eich
  • 4,239
  • 3
  • 21
  • 45
0

When you say:

if(someString) { /* do something */ }

what this means in english is "if the value of someString is not the empty string, do something." In Javascript this is commonly referred to as "truthiness," so the truthiness of any string is true, except the empty string.

When you say:

if(someCondition || someString) { /* do something */ }

written in English this means "if someCondition evaluates to true, or if someString is truthy, then do something." Since "paper" is always truthy, anything || "paper" is always true.

For your purposes, I would re-write the code like this:

var validValues = ["rock", "paper", "scissors"];
if(validValues.indexOf(choice1) >= 0 && validValues.indexOf(choice2) >= 0) {
    /* do something */
}

Basically, make a list of valid values, then check if both choices appear in that list. The game of rock, paper, scissors will never get more valid values, but in other similar situations this list may grow or shrink over time.

Mike Edwards
  • 3,742
  • 17
  • 23