I recently developed a hangman game with HTML, CSS and JavaScript and I would like to get your feedback and tips to improve it. The goal of the game is to guess a secret word before 6 incorrect guesses are completed.
I would like feedback on how I can improve the readability, efficiency and overall structure of the code. I am also open to suggestions on coding practices and standards I should follow.
Game screenshot:
enter image description here
Code:
const BLANKS_TAG = document.getElementById("current-word");
const VIRTUAL_KEYS = document.getElementById("virtual-keys");
const HINT_BUTTON = document.getElementById("hint-button");
const NEW_WORD_BUTTON = document.getElementById("new-word-button");
const HANGMAN_IMAGE = document.getElementById("hangman-image");
const MAX_TRIALS = 6;
let trials = MAX_TRIALS;
let hintUsed;
let blanks;
let words;
let secretWord;
let secretWordArray;
/**
* Updates the hangman image based on the number of attempts remaining.
*/
function updateHangmanImage() {
HANGMAN_IMAGE.src = `img/${MAX_TRIALS - trials}.png`;
}
/**
* Updates the blanks with the correctly guessed letter and marks the
* corresponding span element as correct.
* @param {string} LETTER - The letter to guess.
* @returns {boolean} Returns true if it's a good guess, false otherwise.
*/
function guessAndUpdateBlanks(LETTER) {
const SPANS = BLANKS_TAG.querySelectorAll("span");
let isGoodGuess = false;
let lastCorrectSpan = null;
for (const [I, CHAR] of secretWordArray.entries()) {
if (CHAR === LETTER) {
isGoodGuess = true;
blanks[I] = LETTER;
lastCorrectSpan = SPANS[I];
}
}
if (lastCorrectSpan) lastCorrectSpan.classList.remove("correct");
if (isGoodGuess) return true;
else return false;
}
/**
* Replaces the guessed letters in the string of blanks and updates the hangman image if the guess is incorrect.
* @param {boolean} IS_GOOD_GUESS - Indicates if the guess is correct.
* @param {string} LETTER - The guessed letter.
*/
function replaceGuessedLetters(IS_GOOD_GUESS, LETTER) {
if (IS_GOOD_GUESS) {
const BLANKS_STRING = blanks.join(" ");
const UPDATED_BLANKS_STRING = BLANKS_STRING.replace(
new RegExp(LETTER, "gi"),
`<span class="correct">${LETTER}</span>`
);
BLANKS_TAG.innerHTML = UPDATED_BLANKS_STRING;
} else {
trials--;
updateHangmanImage();
}
}
/**
* Provides a hint by disabling a number of letters that are not in the secret word.
*/
function hint() {
HINT_BUTTON.style.visibility = "hidden";
hintUsed = true;
const VIRTUAL_KEYS_CHILDREN = Array.from(
VIRTUAL_KEYS.querySelectorAll(".btn:not(.disabled)")
);
const MAX_LETTERS_TO_SHOW = Math.floor(Math.random() * 6) + 1;
const INDEXES = [];
while (INDEXES.length < MAX_LETTERS_TO_SHOW) {
const RANDOM_INDEX = Math.floor(
Math.random() * VIRTUAL_KEYS_CHILDREN.length
);
const BUTTON = VIRTUAL_KEYS_CHILDREN[RANDOM_INDEX];
const LETTER = BUTTON.getAttribute("data-value");
if (!INDEXES.includes(RANDOM_INDEX) && !secretWordArray.includes(LETTER)) {
BUTTON.classList.add("disabled");
INDEXES.push(RANDOM_INDEX);
}
}
}
/**
* Checks the game result and displays the secret word accordingly.
*/
function checkGameResult() {
const BLANKS_STRING = blanks.join("");
BLANKS_TAG.textContent = secretWord;
NEW_WORD_BUTTON.style.visibility = "visible";
if (BLANKS_STRING === secretWord) BLANKS_TAG.classList.add("correct");
else BLANKS_TAG.classList.add("incorrect");
}
/**
* Initialize the game and choose a secret word at random.
*/
function initializeGame() {
NEW_WORD_BUTTON.style.visibility = "hidden";
BLANKS_TAG.classList.remove("correct", "incorrect");
VIRTUAL_KEYS.querySelectorAll(".btn").forEach((BUTTON) => {
BUTTON.classList.remove("disabled");
});
secretWord = words[Math.floor(Math.random() * words.length)];
trials = MAX_TRIALS;
blanks = Array(secretWord.length).fill("_");
secretWordArray = secretWord.split("");
hintUsed = false;
BLANKS_TAG.textContent = blanks.join(" ");
updateHangmanImage();
}
/**
* Manages the game event when a virtual key is clicked.
* @param {Event} event - The click event object.
*/
function play(event) {
if (trials === 0 || !blanks.includes("_")) return;
const BUTTON = event.target;
const LETTER = BUTTON.getAttribute("data-value");
const IS_GOOD_GUESS = guessAndUpdateBlanks(LETTER);
replaceGuessedLetters(IS_GOOD_GUESS, LETTER);
BUTTON.classList.add("disabled");
if (trials === 1 && !hintUsed) HINT_BUTTON.style.visibility = "visible";
if (!trials || !blanks.includes("_")) checkGameResult();
}
fetch("./words.txt")
.then((response) => response.text())
.then((data) => {
words = data.split(" ");
initializeGame();
})
.catch((error) => {
console.error(`Error reading word file: ${error}`);
});
document.addEventListener("click", (event) => {
const TARGET = event.target;
if (TARGET === NEW_WORD_BUTTON) {
initializeGame();
} else if (TARGET.parentNode === VIRTUAL_KEYS) {
play(event);
} else if (TARGET === HINT_BUTTON) {
hint();
}
});body {
max-width: 100vw;
max-height: 100vh;
}
header {
margin-top: 20px;
margin-bottom: 20px;
}
h1 {
text-align: center;
}
.game-stage {
min-height: 350px;
max-height: 350px;
display: flex;
align-items: center;
justify-content: center;
}
.game-current-word {
min-height: 50px;
max-height: 50px;
display: flex;
justify-content: center;
font-size: 22px;
font-weight: bold;
}
.virtual-keyboard {
min-height: 80px;
max-height: 80px;
max-width: 100vw;
}
.virtual-keys {
max-width: 600px;
display: flex;
justify-content: center;
align-items: center;
flex-wrap: wrap;
margin: 15px auto;
}
.btn {
background: none;
border: 0;
background-color: #0066cc;
box-shadow: 2px 2px 1px #036;
color: #f3ffff;
cursor: pointer;
font: inherit;
font-weight: bold;
line-height: normal;
overflow: visible;
width: 35px;
height: 35px;
border-radius: 5px;
display: flex;
align-items: center;
justify-content: space-evenly;
padding: 0 10px;
margin: 5px;
}
.hint-button,
.new-word-button {
width: 40px;
font-size: 12px;
visibility: hidden;
}
.disabled {
background-color: #fff;
color: #000;
pointer-events: none;
cursor: not-allowed;
}
.correct {
color: green;
font-size: 25px;
}
.incorrect {
color: red;
font-size: 25px;
}<!DOCTYPE html>
<html lang="en">
<head>
<title>Hangman game</title>
<meta charset="UTF-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<link rel="stylesheet" type="text/css" href="assets/normalize.css">
<link rel="stylesheet" type="text/css" href="assets/style.css" />
<script src="assets/index.js" defer></script>
</head>
<body>
<header><h1>Hangman game</h1></header>
<section class="game-stage">
<button type="button" class="btn hint-button" id="hint-button">Hint?</button>
<img id="hangman-image"/>
<button type="button" class="btn new-word-button" id="new-word-button">New Word</button>
</section>
<section class="game-current-word">
<p id="current-word"></p>
</section>
<section class="virtual-keyboard">
<div class="virtual-keys" id="virtual-keys">
<button type="button" class="btn" data-value="a">A</button>
<button type="button" class="btn" data-value="b">B</button>
<button type="button" class="btn" data-value="c">C</button>
<button type="button" class="btn" data-value="d">D</button>
<button type="button" class="btn" data-value="e">E</button>
<button type="button" class="btn" data-value="f">F</button>
<button type="button" class="btn" data-value="g">G</button>
<button type="button" class="btn" data-value="h">H</button>
<button type="button" class="btn" data-value="i">I</button>
<button type="button" class="btn" data-value="j">J</button>
<button type="button" class="btn" data-value="k">K</button>
<button type="button" class="btn" data-value="l">L</button>
<button type="button" class="btn" data-value="m">M</button>
<button type="button" class="btn" data-value="n">N</button>
<button type="button" class="btn" data-value="o">O</button>
<button type="button" class="btn" data-value="p">P</button>
<button type="button" class="btn" data-value="q">Q</button>
<button type="button" class="btn" data-value="r">R</button>
<button type="button" class="btn" data-value="s">S</button>
<button type="button" class="btn" data-value="t">T</button>
<button type="button" class="btn" data-value="u">U</button>
<button type="button" class="btn" data-value="v">V</button>
<button type="button" class="btn" data-value="w">W</button>
<button type="button" class="btn" data-value="x">X</button>
<button type="button" class="btn" data-value="y">Y</button>
<button type="button" class="btn" data-value="z">Z</button>
</div>
</section>
</body>
</html>Github repository: https://github.com/xSyrax123/hangman
Thank you in advance for your help and constructive comments.
1 Answer 1
JS
It feels a tad weird to me seeing stuff that isn't top-level constants named in ALL_CAPS like that. To be clear, I'm not saying that naming function parameters and local const variables that way is wrong, write how you like, I just find it a bit unusual
Instead of a disabled class, I'd use the element attribute with the same name. That by itself keeps it from triggering events without needing the pointer-events css, and it also makes sure we disable other ways of clicking the button (like, say, keyboard navigation. Try using the tab key to highlight a button, then pressing the space bar a few times). For styling disabled elements, we don't need a class, but can instead use the :disabled selector
guessAndUpdateBlanks
The logic behind whatever is going on with this function and BLANKS_TAG makes very little sense to me. Fortunately, as we immediately move on to replaceGuessedLetters once we're done here, and that function completely replaces the contents of BLANKS_TAG, none of it actually matters. So I say let's just remove that stuff, leaving this function responsible for updating blanks only
Also, low-hanging fruit, but if (isGoodGuess) return true; else return false; just reduces to return isGoodGuess
replaceGuessedLetters
The name feels misleading. If this function's job is only to replace guessed letters, it would do nothing in the event of a bad guess, but this does not seem to be the case. Renaming it to clarify its function could help, but I'd consider splitting it into two handleGoodGuess() and handleBadGuess() functions
hint
The loop does not exit until we've found a certain number of un-guessed letters that are not in the word. This is a problem if the word is something like "subdermatoglyphic", because we might run out of such letters before we've disabled enough of them, and then we'll just keep looping forever. An approach that only loops a fixed number of times, guaranteeing a unique item is disabled each pass (if one exists), could be one way to avoid this. An implementation of that might look a bit like the following:
const CANDIDATES_FOR_DISABLING = new Set(Array.from(document.querySelectorAll("#virtual-keys > button:enabled")).filter(BTN => ! secretWordArray.includes(BTN.getAttribute("data-value"))));
for (let i = 0; CANDIDATES_FOR_DISABLING.size > 0 && i < MAX_LETTERS_TO_SHOW; ++i) {
const BUTTON = Arrays.from(CANDIDATES_FOR_DISABLING.values())[Math.floor(Math.random() * CANDIDATES_FOR_DISABLING.size)];
BUTTON.setAttribute("disabled", "");
CANDIDATES_FOR_DISABLING.delete(BUTTON);
}
HTML
The buttons that make up the virtual keyboard feel a bit repetetive. I'd be tempted to try and generate them programmatically. Perhaps with something like:
for (const CHAR of ALPHABET) {
const BUTTON = document.createElement("button");
BUTTON.setAttribute("data-value", CHAR.toLowerCase());
BUTTON.innerText = CHAR.toUpperCase();
BUTTON.addEventListener("click", EVT => play(EVT));
VIRTUAL_KEYS.append(BUTTON);
}
I also don't see the point of the btn class. It looks like we apply it to all button elements. So why not just query for button-type elements instead of elements with the btn class? The .btn rule in the CSS could just as easily be a button rule, no?
Finally, the "current word" and "hint button" elements already have IDs, so assigning unique classes to them as well feels kind of pointless when it seems like we could just use the IDs instead
<script type="module">, and then useimportandexportto connect them together. Those three modules could contain either aclassor just the functions relevant to that module. I think OOP is good for game programming, but the function-based approach is more popular in web development. \$\endgroup\$