1
\$\begingroup\$

I am new to javascript and I made a simple Rock Paper scissor game with User Interface. There will be 5 rounds and user will play against the computer by clicking any of the three buttons.

See Live preview

Review my code:

Html:

<!DOCTYPE html>
<html>
  <head>
    <meta charset="UTF-8" />
    <title>Rock Paper Scissors</title>
    <link rel="stylesheet" href="style.css" />
  </head>

  <body>
    <h1 class="header">Rock Paper and Scissors</h1>
    <div class="round">Round 1</div>
    <div class="options">
      <button class="rock">Rock</button>
      <button class="paper">Paper</button>
      <button class="scissor">Scissor</button>
    </div>
    <div class="result"></div>
    <div class="score"></div>
    <script type="text/javascript" src="external.js"></script>
  </body>
</html>

Javascript code:

let playerScore = 0,
  computerScore = 0;

//generate random choice
function getComputerChoice(choices) {
  return choices[Math.floor(Math.random() * choices.length)];
}

function playRound(event) {
  //declare a variable playerSelection and store the event nodes class
  let playerSelection = event.target.classList.value;

  //take the computer choice in another variable
  let computerSelection = getComputerChoice(["ROCK", "PAPER", "SCISSOR"]);

  //create two variables that'll reference to divs result and score
  const resultDiv = document.querySelector(".result");
  const scoreDiv = document.querySelector(".score");
  let result;

  //after any condition is met, increment each player's score and update the result
  if (playerSelection != null) playerSelection = playerSelection.toUpperCase();
  if (playerSelection === computerSelection) {
    result = `It's a tie!`;
  } else if (
    (playerSelection === "ROCK" && computerSelection === "SCISSOR") ||
    (playerSelection === "PAPER" && computerSelection === "ROCK") ||
    (playerSelection === "SCISSOR" && computerSelection == "PAPER")
  ) {
    result = `You won ${playerSelection} beats ${computerSelection}`;
    playerScore++;
  } else {
    result = `You lose ${computerSelection} beats ${playerSelection}`;
    computerScore++;
  }

  //update div.result
  resultDiv.textContent = result;
  //update score
  scoreDiv.textContent = `You : ${playerScore} Computer : ${computerScore}`;
}

function playGame() {
  let counter = 0;
  //call the addeventlistener  'click'  and update the score div consequently
  const options = document.querySelector(".options");
  options.addEventListener("click", (event) => {
    const isButton = event.target.nodeName === "BUTTON";
    if (!isButton || counter >= 5) {
      return;
    }
    counter++;
    const round = document.querySelector(".round");
    playRound(event);
    if (counter < 5) {
      round.textContent = `Round ${counter + 1}`;
    } else {
      displayWinner();
    }
  });
}

//after game ends
function displayWinner() {
  const round = document.querySelector(".round");
  if (computerScore > playerScore) round.textContent = "Computer won :/";
  else if (playerScore > computerScore) round.textContent = "You won!";
  else round.textContent = "Nobody won the game ";
}
//calling the function
playGame();
\$\endgroup\$
1

1 Answer 1

1
\$\begingroup\$

General points

  • Try to make the code self explanatory and avoid adding comment that state the obvious.

  • Avoid repeated DOM queries, use variables to store elements once when your code first starts.

  • Isolate your code from the global scope. There are many ways to do this. A tried and true method is to wrap the code in a function that is immediately run. Called an Immediately Invoked Function Expression (IIFE). See rewrite.

  • Try to keep variable names short.

  • Its is a bad habit not to wrap all blocks in {}. Example

    Bad habit

    if (foo) bar = 2;
    

    Good

    if (foo) { bar = 2; }
    
  • Try to make functions generic and reusable. For example the function getComputerChoice which select a random item from an array could be called randomItem. You can add this to a JS file with other random functions that you can use in other project.

  • Store magic values (strings and numbers) in constants. Place these constants in one place.

  • Avoid giving functions too many tasks or not clearly defining the tasks by spreading them between functions.

The DOM and JS

  • Use element IDs to uniquely locate elements rather than the class name.

  • Use data property to store data in the markup.

    Or it is most times best to create game related elements via code rather than as a static page. This give you far greater control over the visuals and associated data. But keeping it simple the rewrite adopts your design.

Rewrite

/* Common reusable functions */
const rndItem = arr => arr[Math.random() * arr.length | 0];
const query = qStr => document.querySelector(qStr);

;(()=>{
    const MAX_ROUNDS = 5;
    const HANDS = ["ROCK", "PAPER", "SCISSOR"];
    const WINS = [{hand: HANDS[0], beats: HANDS[2]}, {hand: HANDS[1], beats: HANDS[0]}, {hand: HANDS[2], beats: HANDS[1]}];
    const isWin = (winHand, beatsHand) => WINS.some(w => w.hand === winHand && w.beats === beatsHand);
    const resultEl = query("#result");
    const scoreEl = query("#score");    
    const roundEl = query("#round");
    query("#options").addEventListener("click", playRound);

    const game = {
        plyScore: 0,
        plyHand: null,
        compScore: 0,
        compHand: null,
        round: 0,
        reset() {
            game.round = game.compScore = game.plyScore = 0;
            game.compHand = game.plyHand = null;
        }
    };
    startGame();

    function startGame() {
        game.reset();
        resultEl.textContent = "New Game. Best out of " + MAX_ROUNDS + " games";
    }
    function displayResult() { 
        var result = "It's a tie!";
        if (isWin(game.plyHand, game.compHand)) {
            result = `You won ${game.plyHand} beats ${game.compHand}`;
            game.plyScore ++;
        } else if (isWin(game.compHand, game.plyHand)) {
            result = `You lose ${game.compHand} beats ${game.plyHand}`;
            game.compScore ++;
        }     
        resultEl.textContent = "Results for round " + game.round  + " " + result;
        scoreEl.textContent = `Scores You: ${game.plyScore} Computer: ${game.compScore}`;
    }
    function displayEndGame() { 
        if (game.round === MAX_ROUNDS) {
            let result = "The match is a draw!";
            result = game.plyScore > game.compScore ? 
                `You win the match! ${game.plyScore} rounds to ${game.compScore}` : result;
            result = game.compScore > game.plyScore ? 
                `Computer wins the match! ${game.compScore} rounds to ${game.plyScore}` : result;
            roundEl.textContent = result;
        }
    }    
    function playRound(event) {
        if (game.round < MAX_ROUNDS && event.target.dataset.hand) {
            game.round ++;
            game.plyHand = event.target.dataset.hand;
            game.compHand = rndItem(HANDS);
            displayResult();
            displayEndGame();
        }
    }
})();
<h1 class="header">Paper Scissor Rocks</h1>
    <div id="round" class="round">Select a hand to play</div>
    <div id="options" class="options">
      <button id="rock" class="rock" data-hand="ROCK">Rock</button>
      <button id="paper" class="paper" data-hand="PAPER">Paper</button>
      <button id="scissor" class="scissor" data-hand="SCISSOR">Scissor</button>
    </div>
    <div id="result" class="result"></div>
    <div id="score" class="score"></div>

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.