6
\$\begingroup\$

Hello this is one of my learning purposes projects, a little bit modified version of a popular game called Hangman.

I am a beginner so if you guys could point out some obvious mistakes/good practices/better solutions or implementations I will be grateful.

Here is a preview hosted on github + repository
And my code :

HTML :

<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <link rel="stylesheet" href="style.css">
    <title>Hangman</title>
</head>
<body>
    <main>
        <div class='header'>
            <h1 class='header__heading'>Hangman</h1>
            <div class="header__lifes">
                <svg class="header__icon icon-1">
                    <use xlink:href='img/sprite.svg#icon-heart'></use>
                </svg>
                <svg class="header__icon icon-2">
                    <use xlink:href='img/sprite.svg#icon-heart'></use>
                </svg>
                <svg class="header__icon icon-3">
                    <use xlink:href='img/sprite.svg#icon-heart'></use>
                </svg>
                <svg class="header__icon icon-4">
                    <use xlink:href='img/sprite.svg#icon-heart'></use>
                </svg>
                <svg class="header__icon icon-5">
                    <use xlink:href='img/sprite.svg#icon-heart'></use>
                </svg>
            </div>
        </div>
        <p class='description'>This is version of the classic letter guessing game called Hangman. You are shown a set of blank letters that match a word or phrase and you have to guess what these letters are to reveal the hidden word.</p>
        <div class="keyword">

        </div>
        <div class="menu">
            <input type="text" class='menu__input' value="" id='test'>
            <p class='menu__message hidden'></p> 
            <button class="menu__btn">Try!</button>
        </div>
        <a href="#" class="cta">Start!</a>
    </main>
    <script src='script.js'></script>
</body>
</html>

JS :

'use strict';

const cta = document.querySelector('.cta');
const header = document.querySelector('.header__heading');
const description = document.querySelector('.description');
const lifesBox = document.querySelector('.header__lifes');
const input = document.querySelector('.menu__input');
const guess = document.querySelector('.menu__btn')

// ###############################
// change layout
cta.addEventListener('click', () =>{
   
    description.style.transition = 'all 1s';
    description.style.opacity = "0";
    
    header.style.transition = 'all 1s';
    header.style.transform="translateX(-150%)";

    lifesBox.style.opacity = "1";
    input.style.opacity = "1";
    guess.style.opacity = "1";

    cta.style.display = 'none';
})

// ###############################
// keyword generator

let keyword;
let wordsLeft ;
const keywordBox = document.querySelector('.keyword');

const generateKeyword = () => {
  // TODO in future add more keywords in a better way
  const keywords = [
    'ability',
    'able',
    'about',
    'above',
    'accept',
    'according',
    'account',
    'across',
    'act',
    'action',
    'activity',
    'actually',
  ];

  const randomNumber = Math.floor(Math.random() * keywords.length);

  keyword = keywords[randomNumber];
  wordsLeft = keyword.length;

  for(let i = 0 ; i<keyword.length ; i++){
      keywordBox.innerHTML += `<div class=keyword__letter-${i}></div>`;
  }
}
generateKeyword();

// ###############################
// game starts

const inputValidation = (str) => {
    const lettersOnly = /^[a-z]+$/i;
    const valid = lettersOnly.test(str) ? true : false; // checking both empty and non number
    return valid;
}

let lifes = 5;
let game = true;


// ###############################
// game ends

const endGame = winOrLose => {
    input.classList.toggle('hidden');
    input.nextElementSibling.classList.toggle('hidden');

    if (winOrLose === 'win'){
        input.nextElementSibling.textContent = "You win";
        input.nextElementSibling.style.color = "green";
    }
    else {
        input.nextElementSibling.textContent = "You lost";
        input.nextElementSibling.style.color = "red";
    }

    guess.textContent = 'Play again'
    game = false;
}

// ###############################
// index finder

const allIndexes = (letter) => { // for keywords with more than 1 letter ex. apple 
    let arr = [];
    for(let i = 0 ; i < keyword.length ; i ++){
        if (keyword[i] == letter){
            arr.push(i);
        }
    }
    return arr;
}

// ###############################
// game

let usedLetters = [];

guess.addEventListener('click',() =>{
    if (inputValidation(input.value) && game && !usedLetters.includes(input.value)){
        if (input.value.length <= 1){ // single letter input 
            if (keyword.includes(input.value)){
                const indexArray = allIndexes(input.value);
                for (let i = 0; i < indexArray.length; i++) {
                    document.querySelector(`.keyword__letter-${indexArray[i]}`).textContent = input.value;
                }
                usedLetters.push(input.value);
                input.value = '';
                wordsLeft-=indexArray.length;

                if (wordsLeft == 0){ // Winning game
                    endGame('win');
                }
            }
            else{
                lifes--;
                if (lifes == 0 ){ // Losing game
                    endGame('lose');
                }
                let life = document.querySelector(`.icon-${lifes+1}`);
                life.style.fill = "#333333";
            }
        }
        else { // word input
            if (input.value == keyword){
                for (let i = 0 ; i < keyword.length ; i++){
                    document.querySelector(`.keyword__letter-${i}`).textContent = keyword[i];
                }
                endGame('win');
            }
            else {
                lifes--;
                let life = document.querySelector(`.icon-${lifes+1}`);
                life.style.fill = "#333333";
            }
        }    
    }
    else if (!game) { // Play again
        guess.textContent = 'Try!'
        game = true;
        input.value = '';
        input.classList.toggle('hidden');
        input.nextElementSibling.classList.toggle('hidden');
        lifes = 5 ;
        for (let i = 0; i < lifes; i++) {
            let life = document.querySelector(`.icon-${i+1}`);
            life.style.fill = "#8F0045";
        }
        keywordBox.textContent = '';
        generateKeyword();
    }
})
Improve this question
\$\endgroup\$
1
  • \$\begingroup\$ Pressing enter should check if the letter is correct, it should not be necessary to press the button with the mouse imo \$\endgroup\$ – Caridorc Nov 14 '20 at 17:27
5
\$\begingroup\$

Prefer CSS rules over JS It helps to separate concerns by putting CSS rules as much in the CSS file as possible - putting lots of rules in the JS can detract from the code's logic, which is likely what one is more worried about. Here, rather than assigning to style properties when the game starts, consider adding a class to the container only, and having CSS rules that only apply when that class exists. For example, change:

cta.addEventListener('click', () =>{
   
    description.style.transition = 'all 1s';
    description.style.opacity = "0";
    
    header.style.transition = 'all 1s';
    header.style.transform="translateX(-150%)";
    // ...
});

to

cta.addEventListener('click', () =>{
  document.querySelector('main').classList.add('started');
});

.started .description {
  transition: all 1s;
  opacity: 0;
}
.started .header__heading {
  transition: all 1s;
  transform: translateX(-150%);
}

This way, when you care about layout, the one place to look at is the CSS, rather than having to go through the JS to see what style properties were assigned when.

Use precise variable names The variable wordsLength confused me until I realized it was referring to a count of letters, not words. Also, you have a keyword variable that contains the word to be picked, but also a keyword class in the DOM that contains the partially filled in letters. This could be confusing - consider giving them completely distinct names.

Allow keyboard only The user will be entering letters on their keyboard. Consider allowing them to submit their guess by pressing the enter key.

keyword__letter is weird - you're only using it to be able to select particular children indicies of the parent container. Consider removing it entirely and instead going through the parent's children property. Eg

document.querySelector(`.keyword__letter-${i}`).textContent

can be

keywordBox.children[i].textContent

Simplify and fix inputValidation:

const inputValidation = (str) => {
    const lettersOnly = /^[a-z]+$/i;
    const valid = lettersOnly.test(str) ? true : false; // checking both empty and non number
    return valid;
}

  • simplifies to

    const inputValidation = (str) => /^[a-z]+$/i.test(str);

  • is a function, so for readability, it should contain a verb, perhaps isInputValid

  • Letters should be guessed one letter at a time for proper Hangman, so consider removing the + from the regular expression - or, you could remove the validation here entirely and instead use a pattern and maxlength attribute in the HTML:

    <input type="text" class="menu__input" pattern="[A-Za-z]" maxlength=1>

Always use ===, never use == or != - sloppy comparison with == has weird rules that a script-writer or script-reader should not have to memorize in order to understand what's going on. Consider using the ESLint rule eqeqeq.

Guess click function is too long - it has lots of if/else branches that make the overall logic hard to understand at a glance. Consider splitting it up. For example, maybe something like:

guess.addEventListener('click', () => {
  if (!game) { // Play again
    resetGame();
    return;
  }
  // Input should be validated from the HTML
  const { value } = input;
  if (usedLetters.includes(value)) {
    // maybe give the user a visual indication that the letter has already been picked here
    return;
  }
  if (value.length === 1) {
    handleSingleLetterInput(value);
  } else {
    handleWordInput(value);
  }
});
Improve this answer
\$\endgroup\$
1
  • \$\begingroup\$ thank you very much \$\endgroup\$ – fluffy Nov 16 '20 at 9:38
4
\$\begingroup\$

Cool game! I like the colours and simplicity.

Additional Features:

Give a list of letters already guessed that were wrong. I have poor memory.

Don't punish user for making the same guess twice (E.G don't lose a heart for guessing 'a' twice, only 1 heart)

Validate the guess, so 'ab' shouldn't do anything. Currently you lose a heart.

Most importantly, show the answer at game over!

Glitch

During my testing, I was able to make infinite guesses and tried every single letter, but could never win the game. I made correct guesses after having 0 lives:

enter image description here

Code suggestions

Currently if your JS loads before the document, there should be errors. I suggest wrapping your code in a load document function. On the same note, it's hard to see the flow of the program. It would be much easier if you had 1 function showing what's happening rather than throughout the whole file.

Declare variables at the top of the page, or in the function they are used. You can separate parts into separate files, or check what standards are popular for this in JS. But try to avoid declaring variables outside of functions, throughout the JS file.

const inputValidation = (str) => {
    const lettersOnly = /^[a-z]+$/i;
    const valid = lettersOnly.test(str) ? true : false; // checking both empty and non number
    return valid;
}

'inputValidation' is not a good name, it does not make sense. 'isInputValid' would make more sense.

The function can be shortened to:

const inputValidation = (str) => {
    const lettersOnly = /^[a-z]+$/i;
    return lettersOnly.test(str); // checking both empty and non number
}

I have to go, but will continue the review later

Improve this answer
\$\endgroup\$
2
  • 1
    \$\begingroup\$ Currently if your JS loads before the document His code works because the <script> tag is located at the bottom of the <body>. \$\endgroup\$ – CertainPerformance Nov 14 '20 at 15:13
  • 1
    \$\begingroup\$ thanks for your feedback and time \$\endgroup\$ – fluffy Nov 14 '20 at 15:44

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.