1
\$\begingroup\$

I have a Hangman object constructor with a couple of methods connected to it.

I would like to know how I should encapsulate the logic.

Can I have logic outside the methods (as I'm doing, like hangman.letters.push(e.key);) on document.onkeydown or should I only call the methods here and they should strictly run the game?

Any logic improvements are welcome!

Thank you very much!

var hangman;

function Hangman() {
  this.words = ["sweden", "brazil", "spain", "germany"];
  this.secretWord = "";
  this.letters = [];
  this.guessedLetter = "";
  this.errorsLeft = 5;
  this.messages = {
    win: "You won!",
    loose: "Game over!",
    guessed: "You already guessed this letter, please try again..",
    notValidLetter: "Please enter a valid letter from a-z",
  };
}

Hangman.prototype._getWord = function() {
  return this.words[Math.floor(Math.random() * this.words.length)];
};

Hangman.prototype._checkIfLetter = function(keyCode) {
  return /^[a-zA-Z]*$/.test(String.fromCharCode(keyCode));
};

Hangman.prototype._checkClickedLetters = function(key) {
  return !this.letters.includes(key);
};

Hangman.prototype._addCorrectLetter = function(key) {
  if (this.secretWord.includes(key)) {
    console.log(`${key} is part of the secret word`);
    for (let ix = 0; ix < hangman.secretWord.length; ix++) {
      if (hangman.secretWord[ix] === key) {
        this.guessedLetter += key;
      }
    }
    if (this._checkWinner()) {
      console.log(this.messages.win);
    }
  } else {
    console.log(`${key} is NOT part of the secret word`);
    this._addWrongLetter();
    if (this._checkGameOver()) {
      console.log(this.messages.loose);
    }
  }
};

Hangman.prototype._addWrongLetter = function(letter) {
  return this.secretWord.includes(letter)
    ? this.errorsLeft
    : (this.errorsLeft -= 1);
};

Hangman.prototype._checkGameOver = function() {
  return this.errorsLeft === 0;
};

Hangman.prototype._checkWinner = function() {
  return (
    this.secretWord
      .split("")
      .sort()
      .join("") ===
    this.guessedLetter
      .split("")
      .sort()
      .join("")
  );
};

document.getElementById("start-game-button").onclick = function() {
  hangman = new Hangman();
  hangman.secretWord = hangman._getWord();
  console.log(hangman.secretWord);
};

document.onkeydown = function(e) {
  if (hangman != undefined) {
    // Check if char is a valid alpha char.
    if (hangman._checkIfLetter(e.keyCode)) {
      // Check if we already
      if (hangman._checkClickedLetters(e.key)) {
        hangman._addCorrectLetter(e.key);
        hangman.letters.push(e.key);
      } else {
        console.log(hangman.messages.guessed);
      }
    }
  } else {
    console.log(hangman.messages.notValidLetter);
  }
};
.game-logo {

}

#start-game-button {
  border: 0;
  box-shadow: 0;
  background: linear-gradient(to bottom, #9DA400, #BAC600);
  color: #fff;
  padding: 20px 40px;
  font-size: 30px;
  border-radius: 5px;
}

#hangman {
  text-align: center;
}
<!DOCTYPE html>
<html lang="en">

<head>
  <meta charset="UTF-8">
  <meta name="viewport" content="width=device-width, initial-scale=1.0">
  <meta http-equiv="X-UA-Compatible" content="ie=edge">
  <link rel="stylesheet" href="stylesheets/styles.css">
  <title>The Hangman</title>
</head>

<body>
  <div style="text-align:center; margin: 50px 0px;">
    <img src="http://www.firstcomicsnews.com/wp-content/uploads/2016/09/Hangman-Logo-600x253.png" alt="" class="game-logo">
  </div>
  <div style="text-align:center; margin: 50px 0px;">
    <button id="start-game-button">START GAME</button>
  </div>
  <div style="text-align:center;">
    <canvas id="hangman" height="800px" width="1200px"></canvas>
  </div>
  <script type="text/javascript" src="canvas.js"></script>
  <script type="text/javascript" src="hangman.js"></script>

</body>

</html>

Improve this question
\$\endgroup\$
1
\$\begingroup\$

Encapsulation

I would like to know how I should encapsulate the logic.

Can I have logic outside the methods (as I'm doing, like hangman.letters.push(e.key); in document.onkeydown or should I only call the methods here and they should strictly run the game?

Ideally no internals should be exposed. Using the hangman object should go more like this:

document.onkeydown = function(e) {
  if (hangman.guessLetter(e.keyCode)) {
    if (hangman.won()) {
      // congrats!
      // -> display complete word
      // -> clean up UI
    } else {
      // good guess!
    }
  } else {
    if (hangman.lost()) {
      // ouch, you lost
      // -> clean up UI
    } else {
      // ouch, you missed, try again
    }
  }
  // -> display word status, for example _we_e_ when "w" and "e" were already found in "sweden"
};

No properties of hangman should be accessed, only methods called.

Usability

The game looks incomplete: the UI shows a large image and a button, but expects input from the keyboard and writes messages to console. It's hard to use like this.

Performance

The implementation of _checkWinner is inefficient. It splits the secret word and the guess to their letters and joins them to compare to each other. This operation is unnecessary for the secret word, because once it's generated at the beginning of the game, it never changes. So it would be better to generate the word with sorted letters up front.

Improve this answer
\$\endgroup\$
1
  • \$\begingroup\$ thank you for your review! Great feedback! The game is not complete so I just console.log the logic so I see that everything is working. The ui part will come later \$\endgroup\$ – Isak La Fleur Oct 27 '17 at 21:38

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.