Plain JavaScript Rock, Paper, Scissors

Question

As part of "The Odin Project" web-dev curriculum, I created a simple "Rock, Paper, Scissors" game. I am hoping to receive feedback on how readable the code is and if the organization is easy to follow.

I'm sure this code is convoluted and there are shorter and easier ways to do this. I have only been writing code for a month, so I am more interested in learning to write easy to read code than on optimization.

Are the variables named well? Is the organization of the code easy to follow? I tried to keep to small functions which each doing one small task. I don't know if it is wise to declare all global variables together at the top, or if they should be spread out between functions. I called all functions in one place at the bottom rather than having the calls spread out.

let humanScore = 0;
let computerScore = 0;
const rockButton = document.querySelector('#rockButton');
const paperButton = document.querySelector('#paperButton');
const scissorsButton = document.querySelector('#scissorsButton');
const scoreTextHuman = document.createElement('p');
const scoreTextComputer = document.createElement('p');
const message = document.createElement('p');
const computerMessage = document.createElement('p');
const userMessage = document.createElement('p');
let container = document.querySelector('#container');
let scoreCard = document.querySelector('#scoreCard');
let userInput;
let currentMessage;
let computerDecision;

let getComputerChoice = () => {
    computerDecision = Math.random();
        if (computerDecision < 0.33) {
            computerDecision = "rock";
        } else if (computerDecision > 0.33 && computerDecision < 0.66) {
            computerDecision = "paper";
        } else {
            computerDecision = "scissors";
        }
}

let displayScore = () => { 
    scoreCard.appendChild(scoreTextHuman);
        scoreTextHuman.textContent = ("Your score: " + humanScore);
    scoreCard.appendChild(scoreTextComputer);
        scoreTextComputer.textContent = ("Computer score: " + computerScore);
}

let removeScoreAndMessages = () => {
    scoreCard.removeChild(scoreTextComputer);
    scoreCard.removeChild(scoreTextHuman);
    scoreCard.removeChild(message);
    scoreCard.removeChild(computerMessage);
    scoreCard.removeChild(userMessage);
}

let displayMessage = () => {
    if (currentMessage === "tie") {
        scoreCard.appendChild(message);
            message.textContent = "It is a tie."
        scoreCard.appendChild(userMessage);
            userMessage.textContent = "You chose: " + userInput;
        scoreCard.appendChild(computerMessage);
            computerMessage.textContent = "The computer chose: " +computerDecision;
    } else if (currentMessage === "win") {
        scoreCard.appendChild(message);
            message.textContent = "Congratulations! You won that round."
        scoreCard.appendChild(userMessage);
            userMessage.textContent = "You chose: " + userInput;
        scoreCard.appendChild(computerMessage);
            computerMessage.textContent = "The computer chose: " +computerDecision;
        } else {
        scoreCard.appendChild(message);
            message.textContent = "Oh no! It seems you lost that one."
        scoreCard.appendChild(userMessage);
            userMessage.textContent = "You chose: " + userInput;
        scoreCard.appendChild(computerMessage);
            computerMessage.textContent = "The computer chose: " +computerDecision;
    }
}

let getUserChoice = () => {
    let target = event.target;

    switch(target.id) {
        case 'rockButton':
            userInput = 'rock';
            switch(computerDecision) {
                case 'rock':
                    currentMessage = "tie";
                    break;
                case 'paper':
                    currentMessage = "loss";
                    computerScore++;
                    break;
                case 'scissors':
                    currentMessage = "win";
                    humanScore++;
                    break;
            }
            break;
        case 'paperButton':
            userInput = 'paper';
            switch(computerDecision) {
                case 'rock':
                    currentMessage = "win";
                    humanScore++;
                    break;
                case 'paper':
                    currentMessage = "tie";
                    break;
                case 'scissors':
                    currentMessage = "loss";
                    computerScore++;
                    break;
            }
            break;
        case 'scissorsButton':
            userInput = 'scissors';
            switch(computerDecision) {
                case 'rock':
                    currentMessage = "loss";
                    computerScore++;
                    break;
                case 'paper':
                    currentMessage = "win";
                    humanScore++;
                    break;
                case 'scissors':
                    currentMessage = "tie";
                    break;
            }
            break;
    }
};

let playRound = (event) => {
    if (humanScore < 5 && computerScore < 5) {
        getComputerChoice();
        getUserChoice();
        displayMessage();
        displayScore();
        container.addEventListener('mousedown', removeScoreAndMessages);
    }
    if (humanScore > 4) {
        alert("Congratulations! You won!");
        location.reload()
    }
    if (computerScore > 4) {
        alert("The computer has won.");
        location.reload()
    }
}   

container.addEventListener('mouseup', playRound);

Answer 1

There are a ton of RPS implementations in JS and even on the code review site. So I will mainly focus on your specific style.

First let's address your questions:

Are the variables named well?

The 3 most important things for naming variables in JS are:

1. It's in camelcase
2. It's in English
3. It's descriptive

Which you have done very well. You can always:

1. Use whitelines to group certain variables together (all buttons, all text,...)
2. You can always default your variable with the type you want to put it in. In your case an empty string for currentMessage and maybe even null for other.

These two tips makes it easier to read your code and know what is happening.

Is the organization of the code easy to follow?

You say this

I tried to keep to small functions which each doing one small task.

Which is good and does make the code more readable. BUT the way you should be thinking is that a function would also work standing on its own. This is pretty hard in your code as you almost use no parameters.

let displayScore = () => { ... }

Could be

let displayScores = (humanScore, computerScore) => { ... }

Before even reading what's inside the function, I now know there will be two scores displayed. If I want to show another score at another point at the code, I don't need to adjust the globals.

Which brings us to the next question:

Declare all global variables together at the top or spread out between functions?

It's useful to define globals if they are used throughout functions. But this has a lot of drawbacks.

One is you lose the connection between the global and your function. For example, you have a global rockbutton, but this doesn't get called anywhere. The more complex your application becomes, the easier this oversight becomes.

Basically you should look up the rules when to use a global and you can see it is mostly config data that does not change throughout the workflow.

Bottom line: It is better to declare your variables at the top of your function and pass them to other function if needed.

Other points

1. You have a small mistake in your if condition.If the computers returns 0.33, it will returns scissors, giving this option a slight more chance to hit.
2. You remove and add messages, which IMO is weird to do. Just replace the textcontent instead of removing the whole element
3. Split up user choice and computer choice into two functions, now it does more logic then needed.
4. Consider looking into JS literals, which is a more readable way to combine strings.
5. Whenever an if else appears and you have more than two possible outcomes, consider to use a switch case instead (like you already did in getUserChoice)
6. Using mouse up and mouse down as eventlistener seems risky to me. Consider using click events on buttons instead.
7. Last but not least, if you want to improve readability, comments are the best and easiest solution to make your code clear!

I think you started off well in functional programming! Good job!