Javascript - Math.random function returns the same result while called in a for loop

Question

I have a function{computerPlay()} to generate a new random number every time it is called. It returns an item from the array declared at the top. It works how I'd like it to when I test it in the console. However, in order for my game to work properly, I've put it in a for loop and it keeps returning the same value unless I refresh the page. I can't find an answer anywhere and this is my first question. I'm sure the code is sloppy so please ignore that. Here is the code.

  let playOptionsArray = ["Rock", "Paper", "Scissors"];
let totalTies = 0;
let playerPoints = 0;
let computerPoints = 0;
let round = 1;

//formats a player's entry so it will match playOptionsArray
  function firstLetterUppercase(string){
    let firstLetter = string.charAt(0);
    let firstLetterCapitalized = firstLetter.toUpperCase();
    let restOfResponse = string.slice(1);
    let formattedResponse = firstLetterCapitalized + restOfResponse;
    return formattedResponse;
  }

//compares player entry to ensure it matches an item on the playOptions array
  function checkPlayerResponse(response){
    if (response === "Rock" || response === "Paper" || response === "Scissors") {
      return response;
    } else {
      return alert("You must choose either 'Rock', 'Paper', or 'Scissor' as your play. Please try again.");
    }
  }

//generates a random option for the computer's play

  function computerPlay() {
    let computerSelection = playOptionsArray[Math.floor(Math.random()*(playOptionsArray.length))];
    return computerSelection;
  }

  let computerSelection = computerPlay();

  function getPlayerSelection() {
    let playerEntry = window.prompt("You are playing the computer in a game of Rock, Paper, Scissors. First player to five points wins. Choose either Rock, Paper, or Scissors as your play", "Rock");
    if (playerEntry != null) {
    let playerEntryLowercase = playerEntry.toLowerCase();
    playerSelection = firstLetterUppercase(playerEntryLowercase);
  } else {
    getPlayerSelection();
  }
    checkPlayerResponse(playerSelection);
    return playerSelection;
  }

  let playerSelection = getPlayerSelection;

//emulates a single round of rock paper scissor
  function singleRound(){
    if (playerSelection === "Rock" && computerSelection === "Scissors"){
      alert('You have won this round. The computer chose ' + computerSelection + '.');
      playerPoints++;
      round++;
    } else if (playerSelection === "Scissors" && computerSelection === "Paper"){
      alert('You have won this round. The computer chose ' + computerSelection + '.');
      playerPoints++;
      round++
    } else if (playerSelection === "Paper" && computerSelection === "Rock"){
      alert('You have won this round. The computer chose ' + computerSelection + '.');
      playerPoints++;
      round++
    } else if (playerSelection === computerSelection){
      alert('You have tied this round. The computer chose ' + computerSelection + '.');
      totalTies++;
    } else {
      alert('Something went wrong. Refresh the page.')
    }
  }

  function updateScore() {
    document.getElementById("player-points").innerHTML = playerPoints;
    document.getElementById("round").innerHTML = round;
    document.getElementById("computer-points").innerHTML = computerPoints;
    document.getElementById("total-ties").innerHTML = totalTies;
  }

//keeps track of player points and ends when a player gets 5 points
 function game() {
    for (round = 0; computerPoints < 5 && playerPoints < 5; round) {
      getPlayerSelection();
      computerPlay();
      singleRound();
      updateScore();
      }
  }

game();

Obligatory random number cartoon dilbert.com/strip/2001-10-25 — j08691
– j08691, Commented Dec 28, 2017 at 21:14
In game you're not doing anything with the returned value from computerPlay. Is that intentional? — Andy
– Andy, Commented Dec 28, 2017 at 21:15
You keep calling computerPlay but your round scoring checks a global variable computerSelection you only set once (the local computerSelection inside the function shadows the global). Congratulations! You just learned that side-effects are evil. Burned hands teach best and whatnot. — Jared Smith
– Jared Smith, Commented Dec 28, 2017 at 21:16
Also let playerSelection = getPlayerSelection; should be let playerSelection = getPlayerSelection(); if you want it to do anything. — Pointy
– Pointy, Commented Dec 28, 2017 at 21:16
Please note, for future questions, that an MVE should consist of clean (Not Sloppy), easy to read code only. Anything not necessary to address the specific issue your question is asking, or to provide a running example, should be deleted. This helps us help you, and want to help you. Sometimes it also reveals the issue itself. Welcome to SO, I hope your issue gets resolved. How to ask, and MVE: Minimal, complete, Verifiable Example — SherylHohman
– SherylHohman, Commented Dec 28, 2017 at 21:26

Kevin Pei · Accepted Answer · 2017-12-28 21:25:16Z

1

Your returned value from computerPlay isn't stored anywhere in the loop. (should note this is happening with your playerSelection too). Try

for (round = 0; computerPoints < 5 && playerPoints < 5; round) {
  playerSelection = getPlayerSelection();
  computerSelection = computerPlay();
  singleRound();
  updateScore();
}

Edit: @kavakava's answer provides some insight into why this is the case.

edited Dec 28, 2017 at 21:25

answered Dec 28, 2017 at 21:20

Kevin Pei

5,8827 gold badges42 silver badges55 bronze badges

Sign up to request clarification or add additional context in comments.

6 Comments

Jared Smith Over a year ago

Although this certainly works, it might be best to explain to the OP what a less error-prone method than updating a global might look like.

Kevin Pei Over a year ago

@JaredSmith I realized shortly after writing my answer that there was indeed a better way by modifying the loop. Please refresh and check my answer again and let me know if there is still an issue. Thanks

Jared Smith Over a year ago

A better way would be to use the return values and ditch the global entirely: singleRound(computerPlay(), getPlayerSelection()) with singleRound modified to take those as parameters instead of checking global state.

Kevin Pei Over a year ago

Agreed that global variables should generally be avoided, and if it were up to me I'd take your approach. For the sake of answering OP's question, however, I think I will keep this answer to explain why his code isn't working and leave better program design for him to learn another day (or from y'all in the comments :) )

Jared Smith Over a year ago

I went ahead and spelled it out in an answer.

|

Jared Smith · Accepted Answer · 2017-12-28 21:35:56Z

Your problem is a combination of two well-known sources of error: variable shadowing and global state.

In JavaScript, each function creates a new scope:

var foo = 3;
function() { var bar = 2; };
console.log(foo + bar); // ERROR, bar is local to the function

Inner scopes can access stuff from outside (you can still see foo inside the function) but not the other way around. Shadowing is what happens when you declare the same name in an inner scope as an outer:

var foo = 3;
(function() {
  var foo = 2;
  console.log(foo); // 2
})();
console.log(foo); // 3 the outer foo isn't change by the inner.

If you omit the declaration (i.e. var, let, const) then the program searches up through scopes to find it:

var foo = 3;
(function() {
  foo = 2; // will reach up to the outer foo
})();
console.log(foo); // 2

If you aren't mixing scopes this way, you can't have this problem. The easiest way to solve it is to prevent it from happening. Modify your singleRound function to take two parameters:

function singleRound(computerSelection, playerSelection) {
  // everything else the same
}

Now when you call it in the loop:

singleRound(computerPlay(), getPlayerSelection());

When you use the return values you can't go wrong as easily.

kavakava · Accepted Answer · 2017-12-28 21:23:40Z

0

function computerPlay() {
    let computerSelection = playOptionsArray[
            Math.floor(Math.random()*(playOptionsArray.length))
         ]; // Line #1

     return computerSelection;
}

let computerSelection = computerPlay(); // Line: #2

This seems to be your problem. The reason it was never changing was because you never changed it. remove the 'let' on line 1 and you should be all set. You were returning the var in computerPlay, not the global.

answered Dec 28, 2017 at 21:23

kavakava

691 silver badge1 bronze badge

Collectives™ on Stack Overflow

Javascript - Math.random function returns the same result while called in a for loop

3 Answers 3

6 Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

6 Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related