Rock, paper, scissors with HTML & CSS & JS

Question

I started studying web development a few months ago and to practice my skills I decided to do the game of rock, paper, scissors.

Here are some questions for review:

• Is the use of HTML semantics correct and how could I improve it?
• How could I improve the CSS and JavaScript code in terms of readability, optimization and best practices?
• Are the class, functions and variable names correct (I am not a native English speaker and I am learning, so there may be things misspelled)?
• Is the style of the code correct?

Game screenshot:

Code:

 let counter_rounds = 0; let player_wins = 0; let computer_wins = 0; let draws = 0; const ROUNDS_PER_GAME = 3; const choices = ["ROCK", "PAPER", "SCISSORS"]; const score_human = document.querySelector(".score-human"); const score_computer = document.querySelector(".score-computer"); const result_round = document.querySelector(".result-round"); const result_human = document.querySelector(".result-human"); const final_result = document.querySelector(".final-result"); const result_computer = document.querySelector(".result-computer"); const image_container_player = document.querySelector(".player-choose .image-container"); const image_container_computer = document.querySelector(".computer-choose .image-container"); const buttons = document.querySelector(".buttons-container"); const new_game_button = document.querySelector(".new-game-button"); buttons.addEventListener("click", play); new_game_button.addEventListener("click", reset_game); function play(event) { counter_rounds++; let player, computer; if ( event.target.classList.contains("btn") || event.target.closest(".button-container") && counter_rounds < 3 ) { const button_value = event.target .closest(".button-container") .getAttribute("data-value"); player = choices.indexOf(button_value); computer = Math.floor(Math.random() * 3); check_winner_round(player, computer); } if (counter_rounds === ROUNDS_PER_GAME) { disable_buttons(); check_winner_game(); new_game_button.style.display = "block"; } } function check_winner_round(player, computer) { if (player === computer) { result_round.textContent = "It's a draw."; result_human.textContent = `You chose ${choices[player]}.`; result_computer.textContent = `Computer choose ${choices[computer]}.`; draws++; } else if ((player-computer+3) % 3 === 1) { score_human.textContent = Number(score_human.textContent) + 1; result_round.textContent = "You won."; result_human.textContent = `You chose ${choices[player]}.`; result_computer.textContent = `Computer chose ${choices[computer]}.`; player_wins++; } else { score_computer.textContent = Number(score_computer.textContent) + 1; result_round.textContent = "You lost."; result_human.textContent = `You chose ${choices[player]}.`; result_computer.textContent = `Computer chose ${choices[computer]}.`; computer_wins++; } update_image_containers(player, computer); } function check_winner_game() { if (player_wins === computer_wins) final_result.textContent = "It's a draw."; else if (player_wins > computer_wins) final_result.textContent = "You won the set."; else final_result.textContent = "Computer wins the set."; } function update_image_containers(player, computer) { const image_player = `img/${choices[player].toLowerCase()}.png`; const image_computer = `img/${choices[computer].toLowerCase()}.png`; image_container_player.style.backgroundImage = `url(${image_player})`; image_container_computer.style.backgroundImage = `url(${image_computer})`; image_container_player.classList.remove("empty"); image_container_computer.classList.remove("empty"); } function reset_image_containers() { image_container_player.classList.add("empty"); image_container_computer.classList.add("empty"); image_container_player.style.backgroundImage = ""; image_container_computer.style.backgroundImage = ""; } function set_buttons_state(disabled) { const buttonContainer = document.querySelector(".buttons-container"); buttonContainer.classList.toggle("disabled", disabled); } function disable_buttons() { set_buttons_state(true); } function enable_buttons() { set_buttons_state(false); } function reset_game() { score_human.textContent = 0; score_computer.textContent = 0; result_round.textContent = ""; result_human.textContent = ""; result_computer.textContent = ""; final_result.textContent = ""; player_wins = 0; computer_wins = 0; draws = 0; counter_rounds = 0; reset_image_containers(); enable_buttons(); new_game_button.style.display = "none"; }

 *, *::before, *::after { box-sizing: border-box; padding: 0; margin: 0; } body { background-color: #000; max-height: 100vh; } header { margin-top: 20px; margin-bottom: 20px; } h1 { color: #f3ffff; text-align: center; } .scoreboard { display: grid; grid-template-columns: repeat(2, 1fr); justify-items: center; align-items: center; background-color: #014689; height: 150px; width: 400px; margin: 0 auto; border: 4px solid #f3ffff; border-radius: 20px; } .player-score, .computer-score { display: flex; flex-direction: column; justify-content: space-around; height: inherit; width: 100px; } .player, .computer { color: #f3ffff; font-weight: bold; font-size: 25px; text-decoration: underline #f3ffff; display: flex; justify-content: center; align-items: center; } .score-computer, .score-human { background-color: #000; border: 4px solid #f3ffff; border-radius: 10px; color: #ea4225; font-family: 'Seven Segment', sans-serif; font-weight: bold; font-size: 55px; display: flex; justify-content: space-around; align-items: center; } .game-board-container { display: flex; justify-content: center; align-items: center; height: 385px; max-width: 100vw; padding-left: 10%; padding-right: 10%; } .game-board { display: flex; justify-content: center; align-items: center; } .player-choose p, .computer-choose p { color: #ffd700; font-weight: bold; font-size: 25px; text-decoration: underline #ffd700; } .circle { background-color: #87ceeb; border: 3px solid #3b83bd; border-radius: 50%; height: 250px; width: 250px; margin-top: 20px; } .image-container { background-repeat: no-repeat; background-size: 80% 80%; background-position: center; height: 100%; width: 100%; } .vs-image { background-repeat: no-repeat; background-size: 100% 100%; background-position: center; height: 200px; width: 200px; } .game-play p { font-family: 'Proxima Nova', sans-serif; font-weight: bold; text-align: center; } .win-condition { color: #f5f5dc; font-size: 18px; } .round-results { color: #f3ffff; font-size: 20px; margin-top: 20px; margin-bottom: 50px; } .choose-option { font-family: 'Proxima Nova', sans-serif; font-weight: bold; font-size: 25px; color: red; } .buttons-container { display: flex; justify-content: center; max-width: 100vw; margin-top: 10px; } .button-container { background-color: #014689; width: 150px; height: 70px; padding: 20px; margin-left: 20px; border-radius: 5px; cursor: pointer; } .btn { background: none; border: 0; color: #f3ffff; cursor: pointer; font: inherit; font-weight: bold; line-height: normal; overflow: visible; width: 100%; height: 100%; display: flex; align-items: center; justify-content: space-evenly; padding: 0 10px; } .btn img { width: 50px; height: 50px; background-color: transparent; } .image-container.empty { background: none; } .new-game-button { background-color: #014689; border-radius: 5px; width: 130px; cursor: pointer; display: none; margin: 0 auto; margin-top: 10px; } .disabled { opacity: 0.5; pointer-events: none; cursor: not-allowed; }

 <!DOCTYPE html> <html lang="es"> <head> <meta charset="UTF-8" /> <meta name="viewport" content="width=device-width, initial-scale=1.0" /> <title>Rock, Paper, Scissors</title> <link rel="stylesheet" href="css/styles.css" /> <link href="https://fonts.cdnfonts.com/css/seven-segment" rel="stylesheet" /> <link href="https://fonts.cdnfonts.com/css/proxima-nova-2" rel="stylesheet" /> </head> <body> <header><h1>Rock, Paper, Scissors</h1></header> <main> <section class="scoreboard"> <div class="player-score"> <p class="player">PLAYER</p> <p class="score-human">0</p> </div> <div class="computer-score"> <p class="computer">COMPUTER</p> <p class="score-computer">0</p> </div> </section> <section class="game-board-container"> <div class="game-board"> <div class="player-choose"> <p class="player">PLAYER</p> <div class="circle"> <div class="image-container empty"></div> </div> </div> <img src="img/vs.png" class="vs-image" /> <div class="computer-choose"> <p class="computer">COMPUTER</p> <div class="circle"> <div class="image-container empty"></div> </div> </div> </div> </section> <section class="game-play"> <p class="win-condition">The best score out of 3 is the winner.</p> <div class="round-results"> <p class="result-human"></p> <p class="result-computer"></p> <p class="result-round"></p> <p class="final-result"></p> <button class="btn new-game-button">New Game</button> </div> <p class="choose-option">Choose an option:</p> <div class="buttons-container"> <div class="button-container" data-value="ROCK"> <button class="btn game-button"> <img src="img/rock_emoji.png" alt="rock_button"> Rock </button> </div> <div class="button-container" data-value="PAPER"> <button class="btn game-button"> <img src="img/paper_emoji.png" alt="paper_button" data-value="PAPER"> Paper </button> </div> <div class="button-container" data-value="SCISSORS"> <button class="btn game-button"> <img src="img/scissors_emoji.png" alt="scissors_button" data-value="SCISSORS"> Scissors </button> </div> </div> </section> </main> <script src="js/script.js"></script> </body> </html>

Github link

Answer 1

1 HTML

Your markup will return 10 flags while validating it. In total, there is 1 red flag (error), 3 yellow flags (warning), and 6 green flags (info).

The red flag is for a missing alt attribute for an image in HTML line 38 which will be covered in section 1.2.3.

The 3 yellow flags are received for missing headlines or wrong usage of a section tags which will be covered in section 1.2.1.

The green flags are received for using a trailing slash in replace elements (tags with no closing tag) which have no use but can have downsides in certain conditions. Since these conditions are not met here, those flags will be no further mentioned or covered.

Additionally, to the flags made by w3c validator, I have to red flag the lang attribute which you used in HTML line 2: <html lang="es"> which in this case indicates the website to be written in Spanish while it is written in English.

1.1 Conventions

I see some approaches to implementing conventions. While they are acceptable I would not agree with all or highly advise to change them or implement others.

1. You use 3 different stylesheets with the usage of the link tag. In the first link, you decided to use a single line, you moved the last 2 into multiple lines highlighting the single attributes. IMHO a single-line link is not less readable but less open to coding errors by missing brackets. In any case, you should decide between both theories and use the same convention on all the links.
2. Your custom CSS comes before the framework/fonts CSS. Custom CSS should always be declared last so that frameworks can not override your custom styles (except specificity weight)!
3. While it is technically correct to use a script tag at the end of the body I would always add them in the head element. To ensure that referenced elements already exist you could either use window.addEventListener('DOMContentLoaded', ... ) or use the defer attribute inside your script tag.
4. While I'm a massive fan of header and main usage, I would recommend indenting the h1 tag into a new line. Block-level elements should always be written in new lines even if they are the only child elements of an element. That improves the readability and recognition of those elements.
5. Add logical gaps (linebreak) between elements to split the block apart. In your case splitting the sections visible apart would improve readability.

1.2 Accessibility & Semantics

You tried to use semantic tags wherever possible. However, you paid no extra attention to accessibility beyond that.

1. Sections should always have a headline ranging from h2 to h6 that also works as a label for that section. If no headline is used, then a standard div would be more appropriate. W3C will flag a missing headline.
2. A p stands for paragraph. It is an element to contain flow text and displays that content in a block form. It is not an appropriate use to use it for single words or the display of a score. In fact, displaying the score is a semantic task for the output element while an output element should have a label. An appropriate code use would be:

<div class="computer-score"> <label class="computer" for="computer-score">COMPUTER</label> <output class="score-computer" id="computer-score">0</output> </div> 

3. An Image needs to have an alt attribute so that screen readers know what the content of the image is or what it is about. It also is used as an alternative text in case an image hasn't been loaded. To address the accessibility point of view I cannot see any reasons to present the images to screen readers at all. There is no reason that a blind person will be explained that there is an image that contains a rock as an icon. It should be excluded to screen readers with the usage of aria-hidden="true" so that screen readers will skip it.
4. I can't see any reason to give a div or an image a data attribute. The data-value attribute should be used on the button itself as it is the element that will be clicked.
5. The value of the alt attribute is not appropriate in this context. For example scissors_button is not a description of the image's content or its task. The image in that case just contains a scissor as an item. It is not appropriate to use the alt attribute to describe in which DOM element it has been used. An appropriate value would be: icon of scissors.

As accessibility was not one of your requirements, I will skip it here.

1.3 Body

There is not much to say about the body element and its content. One of your questions was: "Are the class, functions, and variable names correct (I am not a native English speaker and I am learning, so there may be things misspelled)?". All I can answer to that is, that it does not matter much. Names whether they be classes, variables, etc. must be self-explaining. They should not be chosen for efficiency in code or file size but to be readable to other developers while being self-explaining. If you're from Spain and code in Spanish as you know that colleagues of yours are also Spanish, then it would be valid to use Spanish names. It is not a requirement to choose English names. Beyond that, I would highly advise to not limit it to classes but to use ids wherever possible and appropriate. Many of your elements have a unique scope and would be more appropriate with the usage of an id instead of a class.

2 CSS

Your CSS is well-ordered but repetitive. It could be optimized and with it reduced by at least 40%.

2.1 Accessibility

1. Red text color on a black background can cause trouble to red-color-blind persons. As it is a definite can and not a will be this will not necessarily be an issue but must be kept in mind.
2. You miss a highlighting of a selected element in case a user decides to control and play with a keyboard only.

2.2 Responsivness

You didn't pay any attention to RWD (Responsive Web Design) and in general, your web application will only be playable on a larger monitor screen without touch controls. Especially in Web applications, you should make your application responsive.

One major issue is the usage of <meta name="viewport" content="width=device-width, initial-scale=1.0" /> in HTML which removes the DPR (Device Pixel Ratio) feature which you did not counter in CSS. This will make the web application unusable on mobile devices in terms of UX (User Experience).

As you didn't pay any special attention to RWD and Accessibility, I will skip the review about UX/UI here.

2.3 Structure

As said before, your CSS is well structured in order of ordering the elements.

1. You should not use a general reset of all margins and paddings. Many of them have a purpose, especially for UX and UI and removing either cost readability (UX) or causing the extra effort to re-add them manually (efficiency).
2. You should try not to repeat large portions of code. Preferably not repeat code at all. Much of your code could have been cut out if you would select multiple elements with the same styles and adding all properties and values that are equal. Then select the single elements only for element-specific styles.
3. Especially with colors you repeat a hex color quite often. In that case, you should declare a variable for that color within the :root selector. This would make adjustments and maintenance a lot easier as you would need to change the color only in one place (the root).
4. you work a lot with absolute values such as pixels. You should try to use relative units for RWD and UX reasons.

3 JS

Your programming style is mostly up-to-date and you use modern techniques for the most part. Especially the usage of string literals and textContent is noted.

3.1 Conventions

You make good use of deciding between constants and variables.

1. Constants should always be declared on to while global variables should come afterward.
2. Constants should be written in capital letters to immediately spot them in the code and have a visual difference to variables. You started well with ROUND_PER_GAME while unfortunately stopping afterwards.
3. You used:

const button_value = event.target .closest(".button-container") .getAttribute("data-value"); 

This looks strange at first glance. It would be more readable in a single line. However, that approach is strange to start with which will be further addressed in 3.4.1.

4. Parameters should start with a single or double underscores such as __player and __computer to visually differentiate them from variables.
5. You should use logic gaps within the code. Appropriate would be to use a gap to split let and const into different blocks. Especially your addEventListener should be split from the const as well.
6. You should add more comments. While your naming and coding are self-explaining, carefully placed comments can improve readability and understanding of your codes by other developers or teachers.

3.2 Input Validations

You use no inputs with exception of button presses. As such there is no reason to address that part in this review.

3.3 Logic

For the most part, I can follow your logic but it is flawed. The one flaw I could immediately spot was the following case.

The first Round was won by the Computer.
The second round was by the Player.
The round was a draw

The game message is the following:

You chose ROCK.
Computer chose SCISSORS.
You won.
It's a draw.

You should specially mark the announcing who won the entire set. Such as: " The Set ends with a draw".

And you also need to fix to display both choices if the set end in a draw not skipping the last choices.

3.4 Coding Style

Overall you have a nice style of coding which is mostly modern and clean. This is especially worth mentioning for beginners.

1. As said above, you click a button so you should add the data-value attribute to the button itself. You also do not need getAttribute while using data attributes. The value of that attribute could simply be received by using dataset instead:

const BUTTONS = document.querySelectorAll('button'); BUTTONS.forEach(button => button.addEventListener('click', function(event) { let button_dataValue = event.target.dataset.value; console.clear(); console.log(button_dataValue); }) );

<button data-value="Button A">Button A</button> <button data-value="Button B">Button B</button> <button data-value="Button C">Button C</button> <button data-value="Button D">Button D</button>

1. While you already use classList you still keep using the style function to apply inline styles. Use classList all along and apply and remove the background images through CSS as well.
2. Following code block appears to be repetitive:

const image_player = `img/${choices[player].toLowerCase()}.png`; const image_computer = `img/${choices[computer].toLowerCase()}.png`; image_container_player.style.backgroundImage = `url(${image_player})`; image_container_computer.style.backgroundImage = `url(${image_computer})`; 

This indicates to me that both the computer and player use the same 3 images:

• img/paper.png
• img/rock.png
• img/scissors.png

The easiest adjustment is to make a function that returns the correct URL string literal.

function update_imagecontainers(__player, __computer) { image_container_player.classList.remove('empty'); image_container_computer.classList.remove('empty'); image_container_player.style.background = image_url(__player); image_container_player.style.background = image_url(__computer); } function image_url(__choice) { let url = `img/${__choice.toLowerCase}.png` return url; } 

console.log(image_url("ROCK")); console.log(image_url("PAPER")); function image_url(__choice) { let url = `img/${__choice.toLowerCase()}.png`; return url; }

Beyond that, it is a great test for a loop as the computer and player lines are identical.

3. I see some if/else statements that are kinda slow performing. If you have 2 or more if/else statements then a switch statement would perform faster and more efficiently. In the example, I will just use one of your if/else statements (where you miss curly brackets).

function check_winner_game() { if (player_wins === computer_wins) { final_result.textContent = "It's a draw."; } else if (player_wins > computer_wins) { final_result.textContent = "You won the set."; } else { final_result.textContent = "Computer wins the set."; } } 

That entire block can and should be replaced by a switch statement and combined with a dedicated return function such as above:

function check_winner_game() { final_result.textContent = winner_message(); } function winner_message() { switch (true) { case (player_wins === computer_wins): return "It's a draw."; case (player_wins > computer_wins): return = "You won the set"; case (player_wins < computer_wins): return = "Computer wins the set"; } } 

const FINAL_RESULT = document.querySelector('output'); let player_wins = 2; let computer_wins = 1; function check_winner_game() { FINAL_RESULT.textContent = winner_message(); } function winner_message() { switch (true) { case (player_wins === computer_wins): return "It's a draw."; case (player_wins > computer_wins): return "You won the set"; case (player_wins < computer_wins): return "Computer wins the set"; } } window.addEventListener('DOMContentLoaded', check_winner_game);

<output></output>

4 Summary

You have done a really good job for a beginner. However, you still got stuff to learn. I would recommend you learn about switch statements and the usage of return in JS. Overall to learn and implement conventions and to read into RWD to improve your UI and UX skills.