4
\$\begingroup\$

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.

\$\endgroup\$
3
  • 1
    \$\begingroup\$ It looks quite nice already! Some things to think about: do you want to use the UI to keep track of the game state? This makes the code hard to refactor if you want to change the UI later. Also you could split the code into three modules: 1 - hangman image, 2 - keyboard input, 3 - game logic. \$\endgroup\$
    – Kokodoko
    Commented Jun 20, 2023 at 10:23
  • \$\begingroup\$ @Kokodoko I honestly hadn't thought about it when I made the app, but I think the UI would have to be separate from the game logic as you mentioned. Do you think this code would be better with an object oriented approach? \$\endgroup\$ Commented Jun 20, 2023 at 14:01
  • \$\begingroup\$ In either case you can create three modules <script type="module">, and then use import and export to connect them together. Those three modules could contain either a class or 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\$
    – Kokodoko
    Commented Jun 22, 2023 at 9:15

1 Answer 1

1
\$\begingroup\$

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

\$\endgroup\$

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.