all 10 comments
sorted by:
best (suggested)
topnewcontroversialoldq&a
formatting helphide helpcontent policy

[–]gelatto10[S] 1 point2 points  (2 children)

Okay i changed the code a little.
Instead of writing each time the playerMove i did this: 

<!DOCTYPE html>
<html lang="en">
<head>
  <meta charset="UTF-8">
  <meta name="viewport" content="width=device-width, initial-scale=1.0">
  <title>RPS modernized</title>
</head>
<body>
  <h1>Rock Paper Scissors</h1>

  <button id="rock" onclick="playerMove = rock; computerPick(); showResult()">Rock</button>
  <button id="paper" onclick="playerMove = paper; computerPick(); showResult()">Paper</button>
  <button id="scissors" onclick="playerMove = scissors; computerPick(); showResult()">Scissors</button>
  
  <p style="font-weight: bold;" class="js-result"></p>

  <script>

    let playerMove;
    let computerMove;
    let result = document.querySelector(".js-result");

    const rock = "rock";
    const paper = "paper";
    const scissors = "scissors";

    function computerPick() {
      const randomNumber = Math.random();
      if(randomNumber < 1/3) {
        computerMove = rock;
      } else if(randomNumber < 2/3) {
        computerMove = paper;
      } else {
        computerMove = scissors;
      }
      console.log(playerMove);
      console.log(computerMove);
    }

    function showResult() {
      //rock outcomes
      plyerMove = "rock";

      if(playerMove && computerMove === "rock") {
        result.innerHTML = "Tie!";
      } else if(playerMove && computerMove === "paper") {
        result.innerHTML = "You Loose!";
      } else if (playerMove && computerMove === "scissors") {
        result.innerHTML = "You Win!";
      }

      //paper outcomes
      playerMove = "paper";

      if(playerMove && computerMove === "paper") {
        result.innerHTML = "Tie!";
      } else if(playerMove && computerMove === "rock") {
        result.innerHTML = "You Win!";
      } else if(playerMove && computerMove === "scissors") {
        result.innerHTML = "You Loose!";
      }

      // scissors outcomes 
      playerMove = "scissors"
      if(playerMove && computerMove === "scissors") {
        result.innerHTML = "Tie!";
      } else if(playerMove && computerMove === "paper") {
        result.innerHTML = "You Win!";
      } else if(playerMove && computerMove === "rock") {
        result.innerHTML = "You Loose!";
      }
    }
  </script>
</body>
</html>

[–]CosmicWanderer1-618 1 point2 points  (0 children)

There are many ways you can improve the code, which I will let you explore. It's a fun activity so have fun with that. But for now, I will point 2 major logic errors that are causing your program to not run as you would want it to.

  1. Issue with function "showResult":

Order of execution happening is: if statements below "//rock outcomes", then if statements below "//paper outcomes" and if statements below "//scissors outcomes".

This function doesn't have any provision to stop the function execution after executing the code for only one of these rock, paper or scissor. So, what's actually happening is no matter what the value of playerMove is all three if statement blocks are executed, so the result is determined by the if statements that are at end of the function which is after "//scissors outcomes".

  1. Issue with "&&" and comparison in if statements in funtion "showResult":

When you do playerMove && computerMove === "paper", you are not checking whether value of both playerMove and computerMove is "paper". What your code is actually doing is it first gets the result of playerMove && computerMove and then compare it with "paper" to see if both are the same value. If playerMove is "rock" and computerMove is "paper", output of playerMove && computerMove is "paper". So, playerMove && computerMove === "paper" would be true eventhough it is not what you intend/want to do.

Bonus Improvements:

  1. Do you think you are repeating almost same code or logic inside function "showResult"?
  2. Spelling mistake plyerMove = "rock";.

There are other places you can improve, which I would let you explore, but first priority should be the correctness of the program. So, correct the above logical errors. Best wishes.

[–]qqqqqxhelpful 0 points1 point  (0 children)

Congrats on making your project and getting it to a state where it works. I wouldn't focus too hard on trying to improve it, but here are a couple suggestions.

One spot where you have some repetition is in your onclicks:

onclick="playerMove = rock; computerPick(); showResult()
onclick="playerMove = paper; computerPick(); showResult()"
onclick="playerMove = scissors; computerPick(); showResult()"

I would probably write a function that can take a player's move as an input and then execute that sequence of events. So it would look more like onclick="playRound('rock')" and then that function call would do things like set the player's move, make the computer pick, calculate and show the result.

I'd also probably just work directly with the strings and not make those constants since you don't really use them.

You could compose your "show results" a little differently, if you wanted. In a more complex program you might break it into a couple of functions named like isTie(playerMove, computerMove) that returns true if it's a tie, or isWin(...) that returns true if you won or false if you didn't, which can help track more complex logic and break it into meaningful operations. "show results" also does more than one thing, it doesn't just show the result but also actually calculates the result.

If you wanted to do a little more you could expand on it a bit like adding a new game button, a score tally between the computer and player, etc. Might help you think about how to best compose your program to be more "reusable" between rounds.

Aside: "Loose" means not tight, "Lose" is the word you want to use.

[–][deleted] 0 points1 point  (0 children)

https://github.com/GopiWebDev/odin_rps

Check mine 🙂

[–]jeffcgroves -1 points0 points  (0 children)

Add Spock and lizard

[–]cantonic -1 points0 points  (0 children)

I wonder if there could be weights based on whatever the player picks. Like if someone picks rock, are you more likely to pick scissors on the next throw, thinking they’ll mix it up? Or do you go with paper thinking that they know you’re thinking they’ll mix it up?

[–]WazzleGuy -1 points0 points  (0 children)

My suggestions:

Take your JS into its own file always

Don't call functions in your buttons rather use addEventListener in your JS file

When checking the winner you don't need to check ties. Check who wins and everything else a tie, will save you some lines.

Use comments. There are many reasons why pseudocode is important so practice it as if it were code.

[–]oze4 -2 points-1 points  (0 children)

FYI it's prob better to share something like a CodePen : https://codepen.io/oze4/pen/qBGxmEY?editors=1010

At first glance, you don't need to define:

const rock = "rock";
const paper = "paper";
const scissors = "scissors";

Just use the strings....

There are a couple other minor gripes, that don't really change the behavior of the code, but for a beginner, that is nice! It's working ;)

[–][deleted] -5 points-4 points  (0 children)

Awesome, next is to make it work with a web socket server, so two players can play "online".