Javascript if statement only returns true

Question

I am writing a rock paper scissors game.

console.log for the player shows the selected value.

console.log for the computer shows the selected value.

The if statement to check if the values match always returns true, even if the console log shows them to be different.

I have tried writing it a few different ways but always end up with this issue.

// Player Choice Selection 
var player = function() {
  const playerSelects = "rock";
  return playerSelects;
};
console.log(player());

// Computer Choice Selection
var computer = function() {
  const computerSelects = ['rock', 'paper', 'scissors'];
  let randomSelection = Math.floor(Math.random() * 3);
  return computerSelects[randomSelection];
};
console.log(computer());

// Game 
var game = function(player, computer) {
  if (player == computer) {
    const draw = "its a draw";
    return draw;
  }
};
console.log(game());

Can anyone tell me where I am going wrong with this?

Thanks

`game` takes two arguments and you're giving it zero. Both values are undefined. — tadman, Mar 29 '18 at 16:58
player and computer are both undefined so they are the same which returns true. You want `console.log(game( player(), computer() ));` The player and computer in the game function is the arguments coming in not the functions variables you defined — Huangism, Mar 29 '18 at 16:59

score 1 · Answer 1 · answered Mar 29 '18 at 16:59

1

Game is a function taking two arguments:

 var game = function(player, computer){
    if(player == computer){

Therefore if you call it as

 game()

... player and computer will both be undefined as you pass no value in. Might do:

 game(computer(), player())

...however you should really dont shadow these variable names...

answered Mar 29 '18 at 16:59

Jonas Wilms

132,000
20
149
151

Dammit Jonas you're on fire today buddy – Phiter Mar 29 '18 at 17:00
@phiter roughly 100 rep away from my best day ... Lets see how it goes... (however, these answers actually dont deserve the upvotes) – Jonas Wilms Mar 29 '18 at 17:03

adprocas · Answer 2 · 2018-03-29T17:51:59.820

1

You aren't providing anything to the game() function.

I think this is what you would want.

As mentioned, player == computer isn't intuitive due to the naming as well. One wouldn't expect a player to ever be equal to a computer. It would be better to have something like playerValue == computerValue.

// Player Choice Selection 
var player = function() {
  const playerSelects = "rock";
  return playerSelects;
};

// Computer Choice Selection
var computer = function() {
  const computerSelects = ['rock', 'paper', 'scissors'];
  let randomSelection = Math.floor(Math.random() * 3);
  return computerSelects[randomSelection];
};

// Game 
var game = function(playerValue, computerValue) {
  if (playerValue === computerValue) {
    return "its a draw";
  } else {
    return "It's not a draw";
  }
};

var playerValue = player();
var computerValue = computer();

console.log(playerValue);
console.log(computerValue);
console.log(game(playerValue, computerValue));

If you don't have a need for the function expressions, it might be best to use regular functions, like this:

// Player Choice Selection 
function getPlayerChoice() {
  const playerSelects = "rock";
  return playerSelects;
};

// Computer Choice Selection
function getComputerChoice() {
  const computerSelects = ['rock', 'paper', 'scissors'];
  let randomSelection = Math.floor(Math.random() * 3);
  return computerSelects[randomSelection];
};

// Game 
function playGame(playerChoice, computerChoice) {
  if (playerChoice === computerChoice) {
    return "its a draw";
  } else {
    return "It's not a draw";
  }
};

var playerChoice = getPlayerChoice();
var computerChoice = getComputerChoice();

console.log(playerChoice);
console.log(computerChoice);
console.log(playGame(playerChoice, computerChoice));

edited Mar 29 '18 at 17:51

answered Mar 29 '18 at 17:09

adprocas

1,863
1
14
31

"You don't want to run into issues where you have things named the same in different scopes" is bad advice. You should always use the most appropriate name for your variables and shadowing outer scope variables is usually a good thing. In this case the variables inside the function should be `playerValue` and `computerValue` for consistency with the names of the variables outside of it. Using `Val` sometimes and `Value` others is inconsistent. – Paul Mar 29 '18 at 17:13
Agreed, but using `computer` as a variable scoped outside of the function in a more global way, and then using `computer` as a variable within a function as a parameter is frowned upon. This is what I was referring to, and also why I said `I have changed around names for you as an example.` – adprocas Mar 29 '18 at 17:17
But, my variable names are much better than his and definitely does not deserve a downvote for mentioning to be careful of naming variables. – adprocas Mar 29 '18 at 17:17
@Paulpro, there. I changed it to remove any issues and to keep variable names consistent throughout. You can remove your downvote now. – adprocas Mar 29 '18 at 17:22
I didn't downvote for that reason. Saying to be careful with the names and changing them to be better is great, in fact the name `computer` and `player` in the function were quite awful before since they didn't represent those things, `playerValue` or `playerChoice` would be fine. – Paul Mar 29 '18 at 17:23
The reason I downvoted is for these sentences, which I think are harmful/bad advice: "You don't want to run into issues where you have things named the same in different scopes. They will conflict with each other.". If you remove those sentences I will remove my downvote. If you replace them with something about naming variables appropriately because for example `player == computer` seems like an expression that should never evaluate to be true, but `playerChoice === computerChoice` seems reasonable, I will upvote. – Paul Mar 29 '18 at 17:25
1

Thanks for the help, I can see where I was going wrong now. – Cool Spot Mar 29 '18 at 17:26
Thanks @Paulpro, I agree for your reason for variable names of `player == computer`, and I will add that. But, I was more concerned with a variable that is named the same in a global scope possibly conflicting with a variable of the same name in the function scope. Could there be issues with naming variables the exact same between scopes? I guess that would be the question. If there are no issues with naming variables in different scopes to the exact same thing, then yes, that would be bad advice. – adprocas Mar 29 '18 at 17:29
I think it is generally good advice to name variables the exact same as variables in an outer scope. Especially if they represent the same data. That way the outer-scoped variables become inaccessible (shadowed), which is great because you can't use them by accident in the local function. – Paul Mar 29 '18 at 17:31
So that's your opinion, but it would still be good advise to ensure you don't do something like name an outer scoped variable the same as an inner scoped one, and then expect that the inner scoped variable later be an outer scoped variable, without using `let`. Look at 3b) here... if those variables were called something different, there would be no issues coming up with 3b, which one can easily see issues arising from - https://stackoverflow.com/questions/500431/what-is-the-scope-of-variables-in-javascript – adprocas Mar 29 '18 at 17:34
I changed the wording and removed the scope mention. Maybe it's just because I'm a Java developer and all of my static analysis tools yell at me when I have a member variable named the same as a local variable (other than in getters and setters), but it is pretty subjective. So, although we disagree, I have come to terms with the fact that it is opinionated advice, but certainly not harmful like you suggested. – adprocas Mar 29 '18 at 17:39
Here's what I'm talking about - https://wiki.sei.cmu.edu/confluence/display/java/DCL51-J.+Do+not+shadow+or+obscure+identifiers+in+subscopes – adprocas Mar 29 '18 at 17:44
Yes, it's an opinion. I don't deny that. It is just my opinion that you shouldn't give your variables convoluted names to avoid shadowing variables in higher scopes and that shadowing in general makes code easier to read and maintain because you no longer need to keep track of the variables that have been shadowed when reading the function that shadows them. – Paul Mar 29 '18 at 21:41
IMO that coding style recommendation is wrong and self-contradictory. In one sentence it says there is no ambiguity and then immediately follows it saying that the ambiguity burdens code maintainers and auditors. It is not my opinion that there is no ambiguity, that is a fact. I agree with DCL53-J in that link, but it somewhat contradicts DCL51-J also. Everything in that standard is opinionated though, DCL52-J for example, seems silly to me and the extra vertical space seems like a pretty bad tradeoff. – Paul Mar 29 '18 at 21:41
I think shadowing is only beneficial when you are trying to shadow for that reason. Other than that, I don't think convoluted names need to be used to avoid issues. I think organizing code more efficiently and naming variables in a descriptive way will remove convoluted names, and also remove a need for shadowing to avoid convoluted names. This example is perfect. `player` should be an object with a value getter, and the game function should be more ambiguous. What if he had two players? Would he write another game function to make the variable names match two players? – adprocas Apr 02 '18 at 12:27
Then again, this falling back to shadowing to avoid convoluted names might be more of a JavaScript thing. I've never run into an issue where my variable names are convoluted due to an avoidance of shadowing in Java. In fact, it helps me make better descriptive variable names when my static analysis tool yells at me about this. Usually that's just test code as well, but it does seem to force more descriptive variable names if the issue ever comes up. Maybe JS is different due to the larger number of outer variables and all that jazz. – adprocas Apr 02 '18 at 12:32

score 0 · Answer 3 · answered Mar 29 '18 at 17:11

You've got a few things wrong. That some others have partially fixed, allow me to explain:

// Player Choice Selection 
var player = function() {
  const playerSelects = "rock";
  return playerSelects;
};
// Don't log the result here, because it will change in the game instance.

// Computer Choice Selection
var computer = function() {
  const computerSelects = ['rock', 'paper', 'scissors'];
  let randomSelection = Math.floor(Math.random() * 3);
  return computerSelects[randomSelection];
};
// Don't log the result here, because it will change in the game instance.

// Game
// Don't pass the player and computer functions in, because they're in global scope already.
var game = function() {
  // Call an instance of the player, and computer functions.
  // We do this so that each game we play, has new results.
  var playerResult = player();
  var computerResult = computer();
  // Log each result
  console.log(playerResult, computerResult)

  if (playerResult == computerResult) {
    const draw = "its a draw";
    return draw;
  }
};
console.log(game());

Javascript if statement only returns true

3 Answers3