I had a task to create a simple hangman game using O.O.P. with Javascript. It had to render the puzzle and remaining guesses to the DOM but didn't require any C.S.S.
This is what I created.
This is my index.html
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<title>Document</title>
</head>
<body>
<h2 id="render-puzzle"></h2>
<h2 id="guesses"></h2>
<script src="hangman.js"></script>
<script src="app.js"></script>
</body>
</html>
This is my hangman.js which is used to create a hangman object
"strict";
const Hangman = function (word, remainingGuesses) {
this.word = word.toLowerCase().split("");
this.remainingGuesses = remainingGuesses;
this.guessedWords = new Set();
this.status = "Playing";
};
Hangman.prototype.getPuzzle = function () {
let puzzle = [];
this.word.forEach((char) => {
this.guessedWords.has(char) || char === " "
? puzzle.push(char)
: puzzle.push("*");
});
return puzzle.join("");
};
Hangman.prototype.makeGuess = function (guess) {
guess = guess.toLowerCase();
const isUnique = !this.guessedWords.has(guess);
const isBadGuess = !this.word.includes(guess);
if (isUnique) {
this.guessedWords.add(guess);
}
if (isUnique && isBadGuess && this.status === "Playing") {
this.remainingGuesses--;
}
this.calculateStatus();
};
Hangman.prototype.calculateStatus = function () {
const finsished = this.word.every((letter) => this.guessedWords.has(letter));
if (this.remainingGuesses === 0) {
this.status = "Failed";
} else if (finsished) {
this.status = "Finished";
} else {
this.status = "Playing";
}
};
Hangman.prototype.getStatus = function(){
let message = "";
if (this.status === "Playing") {
message = `Remaining Guesses: ${this.remainingGuesses}`
} else if(this.status === "Failed") {
message = `Nice try! The word was ${this.word.join("")}`;
} else {
message = `Great Work! You guessed the word!!!`
}
return message;
}
and this is the app.js where I create an instance of the hangman game
"strict";
const puzzle = document.querySelector("#render-puzzle");
const guesses = document.querySelector("#guesses");
const hangman = new Hangman("Cat", 2);
generatePuzzleDom();
window.addEventListener("keypress", (e) => {
hangman.makeGuess(e.key);
generatePuzzleDom();
console.log(hangman.status);
});
function generatePuzzleDom() {
puzzle.innerHTML = hangman.getPuzzle();
guesses.innerHTML = hangman.getStatus();
}
2 Answers 2
JavaScript is a powerful and unique OO language that dropped a lot of the formality used in languages like Java and C# in favor of expressive coding (creative for want of a better term)
Many people confuse syntax with OO, and will consider code OO if the traditional syntax is involved. Using this
, new
does not make it Good OO. Syntax has little to do with OO, it is the structure of the code that is important.
In JavaScript good OO focuses on Encapsulation, and uses Polymorphism to replace Inheritance.
Your code does not use Inheritance so not much to review on that part.
Your code however does not use any from of safe encapsulation, leaving the object state at the mercy of bad code. Safe encapsulation is the most important part of OO design letting you trust the state of the code and know it will always work no matter the environment.
Reduce code noise
- Function blocks do not need to be terminated with
};
- There is no need to split the string into an array. String have array like properties in JavaScript.
- The keyword
this
is both a danger (can be replaced by code outside the Object) and noisy as it turns up all over the place. In Good OO JavaScript you seldom need to usethis
. The example does not usethis
window
is the default this. Avoid using it randomly.- Learn to use the ternary operator
?
as it can remove a lot of noise from the code. - Unique to JavaScript is the fact that expressions and statements are evaluated in a known order. This introduces the ability to use the shortcircuit style to replace the messy and rather old fashioned
if () { } else if () { }
See example there is only one if statement in the whole thing. - The prototype can be assigned with an object literal eg
eg
Hangman.prototype = {
getPuzzle() { },
makeGuess() { },
... etc ...
}
OO coding
- Use getters and setters
- Hangman is a only ever used as a single instance. I will not work if you create a second instance. Because of this you should not define it as function but rather as a object literal. To protect the state use a IIF to assign the object literal and encapsulate its state.
- Good OO code always protects the object state and does not expose anything that is not needed to use the Object. In javascript we use closure to hold an Objects state and define an object as an interface to that state.
Conflicting needs.
Modern web pages have 3 Main components. HTML, CSS, JavaScript. Unfortunately each was defined by a separate set of designers, each of whom created their own standards for naming.
The result is a complete mess, where HTML, and CSS naming conventions are in direct conflict with JavaScript names. For example "render-puzzle" is not a valid name in JavaScript.
You do not have to follow the naming conventions of CSS and HTML. You can use the JavaScript naming convention and thus reduce the needless overhead of querying the DOM of elements defined by id
. Element ids are automatically created when needed in javascript providing a far simpler method of interfacing with the DOM
Rewrite
The rewrite create hangman as a static instance. Rather than creating a new instance to play a game you call Hangman.newGame(word, guesses)
;
The interface to Hangman then uses getters and setters to provide the functionality needed to play the game.
"use strict";
const Hangman = (() => {
const STATUS = {
playing: 1,
finished: 2,
failed: 3,
};
const statusStrs = {
[STATUS.playing]() { return "Remaining Guesses: " + guesses },
[STATUS.finished]() { return "Great Work! You guessed the word!!!" },
[STATUS.failed]() { return "Nice try! The word was " + word },
};
var word, guesses, status, used;
function calculateStatus() {
const finsished = [...word].every(char => used.has(char));
status = !guesses ? STATUS.failed : finsished ? STATUS.finished : STATUS.playing;
}
return {
get puzzle() {
return word.replace(/./g, chr => used.has(chr) || chr === " " ? chr : "*");
},
set guess(char) {
char = char.toLowerCase();
if (!used.has(char)) {
used.add(char);
!word.includes(char) && status === STATUS.playing && (guesses--);
calculateStatus();
}
},
get usedLetters() { return [...used.values()].join("") },
get status() { return statusStrs[status]() },
newGame(newWord, numGuesses) {
word = newWord.toLowerCase();
guesses = numGuesses;
status = STATUS.playing;
used = new Set();
},
};
})();
;(()=>{
Hangman.newGame("Testing", 5);
renderPuzzle();
addEventListener("keypress", e => {
Hangman.guess = e.key;
renderPuzzle();
});
function renderPuzzle() {
puzzleWords.textContent = Hangman.puzzle;
usedLetters.textContent = Hangman.usedLetters;
guesses.textContent = Hangman.status;
}
})();
<h2 id="puzzleWords"></h2>
<h2 id="usedLetters"></h2>
<h2 id="guesses"></h2>
-
\$\begingroup\$ amazing feedback and detailed explaination. I think what was new to me was that I didn't know you could chain ternary operators like the following,
status = !guesses ? STATUS.failed : finsished ? STATUS.finished : STATUS.playing;
\$\endgroup\$user6248190– user62481902020年12月30日 09:09:19 +00:00Commented Dec 30, 2020 at 9:09 -
\$\begingroup\$ also what is the reason for using
var
instead oflet
andconst
to defineword, guesses, status, used
;? \$\endgroup\$user6248190– user62481902020年12月30日 09:10:39 +00:00Commented Dec 30, 2020 at 9:10 -
1\$\begingroup\$ Some argue "In ES6, there's no reason to use
var
- useconst
instead (orlet
when you must reassign)" and "(If you're writing in ES2015, never usevar
. Linting rule)" - citing linting rule no-var \$\endgroup\$2020年12月30日 18:17:05 +00:00Commented Dec 30, 2020 at 18:17 -
1\$\begingroup\$ @Blindman67 Thanks for your opinions - while I'll continue to disagree, I always like hearing opposing viewpoints to better understand why people code different ways - there's usually something to learn from them :). \$\endgroup\$Scotty Jamison– Scotty Jamison2020年12月30日 20:30:52 +00:00Commented Dec 30, 2020 at 20:30
-
1\$\begingroup\$ @ScottyJamison Yes I totally agree, if there were no opposing view points then change would be very slow. Not a good thing for the forward edge of technology driven work IMHO. :) \$\endgroup\$Blindman67– Blindman672020年12月30日 21:59:31 +00:00Commented Dec 30, 2020 at 21:59
Good things
- The code appears to function properly.
- The code makes good use of strict equality operators and functional techniques like using
.every()
. const
is used for most variables.- Prototypes are used properly.
Suggested changes
Strict mode flaw
Strict mode is enabled with the string literal:
"use strict";
Yet the first line of the two JS files is
"strict";
Variable declarations
Some variables declared with let
could be declared with const
since they are not re-assigned - e.g. puzzle
since it is never re-assigned. Using const
instead of let
helps avoid accidental re-assignment and other bugs.
status values
The values for status
could be stored in constants - e.g.
const STATUS_FAILED = 'Failed';
const STATUS_PLAYING = 'Playing';
// ... etc...
Then those can be used in the code- e.g. instead of
this.status = "Playing";
it can use the constant:
this.status = STATUS_PLAYING;
Those could also be stored in "an enum"
const STATUS_VALUES = Object.freeze({ FAILED: 0, PLAYING: 1, FINISHED: 2});
With this approach, there is no risk of mistyping the values throughout the code, and if a value needs to be updated, it can be done in one place.
Selecting elements
It isn't wrong to use querySelector
to get elements by id but using getElementById()
"is definitely faster" 1 (see this jsPerf test for comparison).
ES-6 class syntax
The code could be converted to the newer ES6 class syntax - bear in mind that it is "primarily syntactical sugar over JavaScript's existing prototype-based inheritance". If there were many subclasses then it would help simplify setting up the prototypal inheritance.
-
\$\begingroup\$ thank you for the feedback, it was very helpful and insightful \$\endgroup\$user6248190– user62481902020年12月30日 09:06:46 +00:00Commented Dec 30, 2020 at 9:06
Explore related questions
See similar questions with these tags.