3
\$\begingroup\$

I have created a pretty simple rock-paper-scissors game that uses event listeners and manipulates the DOM. I would appreciate any constructive feedback. Whether it is readability, variable names, security, literally ANYTHING would be greatly appreciated.

My code can be found here: https://github.com/alvinalic1/RockPaperScissors.git

<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <title>Document</title>
    <link rel="stylesheet" href="style.css">
</head>
<body>
    <div class = title>
        <h1>ROCK ----- PAPER ----- SCISSORS</h1>
    </div>

    <div class="score">
        <h2>SCORE</h3>
        <h3 id="your-score">You: 0 </h3>
        <h3 id="comp-score">Computer: 0 </h3>
    </div>

    <div class="player">
        
    
        <div class = "you">
            <h2 class="your-choice">YOUR CHOICE</h2>
            <img src="images/QuestionMark.jpg">
            <div class = "buttons">
                <button id = "rock">ROCK</button>
                <button id = "paper">PAPER</button>
                <button id = "scissors">SCISSORS</button>
            </div>
        </div>

        <div class = "computer">
            <h2>COMPUTER CHOICE</h2>
            <img src="images/QuestionMark.jpg" id="comp">
        </div>

    </div>
    
    <script src="javascript.js"></script>
</body>
</html>



//array holding possible choices
const RPS = ["Rock", "Paper", "Scissors"];
let playerChoice = "";
let computerScore = 0;
let playerScore = 0;

/*
Method that will get a random number and use that number to access one of the choices in the RPS Array
*/

const computer = document.querySelector(".computer");
const compImg = document.querySelector("#comp");
function getComputerChoice()
{
    let number = Math.floor(Math.random()*RPS.length)
    if(RPS[number] == "Rock"){
        compImg.src="images/rock.jpg";
    }
    if(RPS[number] == "Paper"){
        compImg.src="images/paper.jpg";
    }
    if(RPS[number] == "Scissors"){
        compImg.src="images/Scissors.jpg";
    }
    return RPS[number];
}



/*
Compares the computer choice with the player choice
returns winner
*/
function playRound(getComputerChoice, getPlayerChoice)
{
    let player = getPlayerChoice.toLowerCase();
    let computer = getComputerChoice.toLowerCase();

    if(computerScore == 5){
        alert("GAME OVER COMPUTER WINS");
    }else if(playerScore == 5){
        alert("GAME OVER YOU WIN");
    }

    if(player === computer)
    {
        return "TIE GAME";
    }

    if((computer == "scissors" && player == "rock") || 
    (computer == "rock" && player == "paper") || 
    ( computer == "paper" && player == "scissors")){
        computerScore++;
        // const node = document.createTextNode(`${computerScore} times`)
        // compScore.appendChild(node);
        compScore = document.getElementById("comp-score");
        compScore.innerHTML = ("Computer: " + computerScore);
        return `You LOSE :( ${player} beats ${computer}`;
    }else{
        playerScore++;
        urScore = document.getElementById("your-score");
        urScore.innerHTML = ("You: "+playerScore)
        return `You WIN! ${computer} beats ${player} `;
    }

    
}



const questionMark = document.querySelector("img");

const rock = document.querySelector("#rock");
const paper = document.querySelector("#paper");
const scissors = document.querySelector("#scissors");
const buttons = document.querySelector(".buttons");


const you = document.querySelector(".you");
const img = document.querySelector("img");

var urScore = document.querySelector("#your-score");
var compScore = document.querySelector("#comp-score");

/*
The next three methods are all meant to update the image so it coencides
with the player choice
*/
function showRock(){
    img.src="images/rock.jpg";
    you.insertBefore(img, buttons);
}

function showPaper(){
    img.src="images/paper.jpg";
    you.insertBefore(img, buttons);
}

function showScissors(){
    img.src="images/scissors.jpg";
    you.insertBefore(img, buttons);
}


/*
The next 3 methods are all intended to be even listeners
Once the player picks their choice
The image updates and the score is displayed
*/
rock.addEventListener("click", () =>{
    playerChoice = "rock";
    showRock();
    console.log(playRound(playerChoice, getComputerChoice()));
})


paper.addEventListener("click", () =>{
    showPaper();
    playerChoice = "paper";
    console.log(playRound(playerChoice, getComputerChoice()));
});

scissors.addEventListener("click", () =>{
    playerChoice = "scissors";
    showScissors();
    console.log(playRound(playerChoice, getComputerChoice()));
});
\$\endgroup\$

2 Answers 2

Reset to default
3
\$\begingroup\$

Hooked on a Feeling

In @Alex Answer

Is there a way to implement this, so that such an extension would be simple?

and OP's comment:

functions that are essentially doing the same thing ... figuring out a way to condense that would allow me to implement extensions in an easier way

recognizes that merely re-arranging existing code won't make it extendable.

DSL

computer is just another Player. What's missing is the "Player" concept - an abstraction encapsulating {player and computer}. Then that idea implemented with objects because objects are inherently extendable structures.

DSL Implementation enables Extension

That the Player.name() -> computer must Player.chooseSign() is the same method CALL for Player.name() -> Helen. i.e Player.chooseSign()

A given object containing all necessary state (properties) and behavior (functions) is a complete, self-contained API easily used in an evolving program.

P.S. I wonder if the showXXX methods and related event handlers could coalesce into instances of one object because the only real difference is "rock", "paper", "scissors" essentially.

P.P.S. In the above, perhaps only one handler is needed. Pass in a "target" parameter - the Player maybe? - and get the details from that - Player.choice for example.

Polymorphism is a kind of Extension

let computer = new Player(name, uniqueChooseFunction) - Pass in different functions for human and computer players.

Encapsulation enables Extension

When every actual player is-a Player tic-tac-toe game code will smaller/simpler because everyone is a Player

let players = [player1, player2];
let winner;

players.each(player -> player.chooseSign())
winner = whoWon(players); // score updates

Composition (aka dependency injection) enables Extension

With polymorphic Players, UI code can be generalized in kind, and be less complex. For example a template div block as an object that takes a Player as a parameter.

\$\endgroup\$
2
\$\begingroup\$

What you should ask yourself:

If you wanted to extend this to "rock, paper, scissors, lizard, Spock", how much code would you have to change? Is there a way to implement this, so that such an extension would be simple?

The toy example only will get you so far, but I hope you understand where this is leading.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ This is actually really helpful. While looking over my code I noticed that there were functions that are essentially doing the same thing (my show functions and my eventlisteners)I feel like figuring out a way to condense that would allow me to implement extensions in an easier way. Thank you. \$\endgroup\$
    – snbmt21
    Commented May 14 at 14:13

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.