Typing King - a JavaScript typing game

Question

This is what I made today and I'm keen for a review of my code to see what things I can do better or smarter. I don't know if its relevant but it took 2 hours to code.

Is it smart to use OOP for a project like this?

I have included all the code in the snippet.

var inGame = false;
var currentWord, amountOfErrors, goodAnswers, previousWord;
var timer, time;


$(document).ready(function() {
  //Check if enter is pressed
  $(document).keypress(function(e) {
      if(e.which == 13) {
          enterWord();
      }
  });

  $("#startBtn").click(function () {
    //Start the game when user clicks
    startGame();
    $("#startBtn").hide();
  })

});


function startGame() {
  //Initialize start
  inGame = true;
  document.getElementById("wordField").focus();

  //Reset all variables
  amountOfErrors = 0;
  corrections = 0;
  goodAnswers = 0;
  $("#currentWord").html("");
  $("#errors").html("");
  $("#wpm").html("");

  pickWord();

  //start the timer
  $("#timer").show();
  startTimer();


}

function stopGame () {

  //Stop the timer
  clearInterval(timer);
  $("#timer").hide();
  $("#startBtn").show();
  $("#wordField").val("");

  //Set game to stopped
  inGame = false;

  //Show player his results
  $("#currentWord").html("<strong>Done...</strong>");
  $("#errors").html('<strong>Amount of errors: '+amountOfErrors+'</strong>');
  $("#wpm").html('<strong>Words per minute: '+goodAnswers+'</strong>');

}

function startTimer() {

  //Reset timer back to 60 seconds
  time = 60;

  //Every 100ms the timer gets updated
  timer = setInterval(function () {
    if (time <= 0) {
      stopGame();
    }
    time = time +- 0.1;
    $("#timer").html("<strong>"+time.toFixed(2)+"</strong>")
  },100);

}

function enterWord () {
  //If player is not in game
  if (inGame == false) {
    //Do nothing
  } else if (inGame == true) { //Is in game
    
    checkWord();
  }
}

function checkWord () {
    var enteredWord = $("#wordField").val();

    //Check if the player typed the correct string
    if (enteredWord == currentWord) {
      //if true
      fadeColor("wordField", "backgroundColor", "rgb(43,234,81)");
      $("#wordField").val("");
      pickWord();
      goodAnswers++;
      previousWord = currentWord;

    } else {
      //if false
      amountOfErrors++;
      fadeColor("wordField", "backgroundColor", "rgb(234,43,43)");
    }
}
function pickWord () {

  //Checking how many words ther are in the array
  var aantalWoorden = woorden.length;
  //Amount of words + 1
  aantalWoorden = aantalWoorden + 1;

  //Pick a random number between the amount of words and 0
  var randomWord = Math.floor((Math.random() * aantalWoorden) + 0);

  //Select a random word from the array
  currentWord = woorden[randomWord];

  //Check if its not the same as previous word
  if (currentWord == previousWord || currentWord == undefined) {
    pickWord();
  } else {
    //Show the word
    $("#currentWord").html("<strong>"+currentWord+"</strong>");
  }
}

var woorden = ["value" , "meanwhile", "finally" , "base" , "spikes" ,"creature", "comforts" , "advanced" , "the snout" , "opposite", "the spoon" , "the dust cloud" , "scrape" , "natural" , "significantly" , "the hedgehog" , "create" , "offset" , "enlarge" , "treacherous" , "the direction" , "the culture" , "the territory", "the timetable"];


function fadeColor(id, property, color) {
    var oProperty = $('#'+id+'').css(property);

    $('#'+id+'').css(property, color);
    setTimeout(function() {
      $('#'+id+'').css(property, oProperty);
    },300);
}

@import url('https://fonts.googleapis.com/css?family=IBM+Plex+Mono');

body {
  margin: 0;
  background-color: rgb(234,43,43);
  color: white;
  font-family: IBM Plex Mono;
}

.centerDiv {
  display: flex;
  flex-direction: column;
  margin: auto;
  width: 100%;
  height: 100%;
  align-items: center;
  text-align: center;
}

.headerDiv {
  display: flex;
  flex-direction: column;
  margin: auto;
  width: 100%;
  height: 100%;
  align-items: center;
  text-align: center;
  margin: 10px;
  margin-bottom: 10%;
}

.header {
  font-size: 25px;
}

.textfield {
  padding: 10px;
  border-radius: 3px;
  background-color: white;
  color: black;
  text-decoration: none;
  border: none;
  margin: 10px;
  font-size: 25px;
  transition: all 0.15s ease;
}

.textToType {
  font-size: 25px;
  margin: 10px;
}

.button {
  padding: 10px;
  border-radius: 3px;
  background-color: white;
  color: rgb(234,43,43);
  text-decoration: none;
  border: none;
  margin: 10px;
  font-size: 25px;
  transition: all 0.15s ease;
}
.button:hover {
  cursor: pointer;
  transform: scale(1.05);
}

<!DOCTYPE html>
<html>
<head>
<title>Typing King</title>
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<link rel="stylesheet" href="css/style.css">
<link rel="icon" href="css/favicon.png" type="image/png">
<script src="https://code.jquery.com/jquery-3.3.1.min.js"></script>
<script src="js/typing.js"></script>
</head>
<body>

<div class="centerDiv">

  <div class="headerDiv">
    <a class="header">Typing King</a>
    <a>Are you the next typing king?</a>
  </div>

  <div class="centerDiv">
    <a class="textToType" id="currentWord"></a>
    <input class="textfield" id="wordField" placeholder="Your input"/>
    <a class="button" id="startBtn">Start</a>
    <a id="timer"></a>
    <a id="wpm"></a>
    <a id="corrections"></a>
    <a id="errors"></a>
  </div>

</div>

</body>
</html>

Answer 1

I should start by saying that the game is quite interesting, and for a beginner the code is good, but of course there is plenty of room for improvement.

Small details

You have many small things that may a bit odd or unnecessary even, such as:

• The inGame check

  if (inGame == false) {
  
   //Do nothing
  } else if (inGame == true) { //Is in game  
     checkWord();
  }
  

  Don't use an if or else block you don't have code for it. Not only it makes the code longer as it creates confusion as to what is its purpose. In this case you can cut the first if off and leave only what you had in the else:

  if (inGame == true) { //Is in game  
     checkWord();
  }
  

  You may even use the strict equality if you want to make sure both operands match in type, which would become if (inGame === true). As a side note there isn't much benefit in making such an obvious comment like "Is in game". Save your comments for more relevant things.

• Adding the time:

  time = time +- 0.1;
  

  There is no reason for the + to be there and it makes the instruction harder to read. Just subtract directly: time = time - 0.1;

• Generating the random word:

  Math.floor((Math.random() * aantalWoorden) + 0);
  

  Adding 0 here changes nothing and again adds clutter to the code, and the extra ( and ) that isolate Math.random and aantalWoorden are also not necessary.

• Fading elements:

  var oProperty = $('#'+id+'').css(property);
  

  More or less as some other things i already stated. The + '' is redundant and should be removed.

• corrections doesn't seem to be used at all:

  function startGame() {
      ...
      corrections = 0;
  

  Even though there is an <a> tag for this name, it doesn't seem to be used in any place.

• Mixed languages and random generation

  var aantalWoorden = woorden.length;
  aantalWoorden = aantalWoorden + 1;
  var randomWord = Math.floor((Math.random() * aantalWoorden) + 0);
  

  Some variables are in dutch while others are in english. Try to be consistent and stick to one language.

  The random generation is not correct. Math.random generates a number from 0 to 1 exclusive, so 1 is never actually generated. This means that when Math.floor is applied it rounds the value down and as such the length value is never generated, resulting in the exact intended range.

  Take this simple example:

  const nums = [10, 20, 30];
  const randIndex = Math.floor(Math.random() * nums.length);
  

  nums.length is 3 so Math.random() * 3 will generate a value from 0 to 3 exclusive, that when floored will be a range from 0 to 2 inclusive. This is the intended range for the array in question.

Refactoring

When you see yourself doing the same thing many times, it is time to think again and try to refactor that part because there certainly is a better way. You have very few code that represents repeated logic, but lets take a look at the fadeColor function:

function fadeColor(id, property, color) {
    var oProperty = $('#'+id+'').css(property);

    $('#'+id+'').css(property, color);
    setTimeout(function() {
      $('#'+id+'').css(property, oProperty);
    },300);
}

Note how you are constructing the same selector 3 times with the $('#'+id+''). So this not only repeats the selector construction as it also fetches the element from the DOM multiple times. Save the fetched element in a variable and then use it:

function fadeColor(id, property, newColor) {
    const element = $('#'+id);
    const oldColor = element.css(property);

    element.css(property, newColor);
    setTimeout(function() {
      element.css(property, oldColor);
    },300);
}

Note how i used const instead of var. With ES6 you now have both const and let as other ways of declaring variables. In this case i know i don't want to change element or oldColor, and with const it will give me an error if i accidentally change them. This builds on writing defensive code that gives you errors sooner rather than later.

The setTimeout itself can also be shortened with an Arrow Function:

setTimeout(() => element.css(property, oldColor), 300);

Which makes it super compact and easy to read.

Objects

One downside of the way you have your code structured is that you have multiple global variables. This creates difficulties in managing state and also opens the possibility of colliding with already existing global variables. Those may either be from the window object or even some other plugin/library that gets included up top.

All the game logic can be moved into one or multiple objects/classes, which helps you separating it from the presentation in the page. Sure you can consider this a bit too much for a such a simple game but it's definitely a way to improve especially if you want to keep adding features.

As a first step you could consider the following approach:

function Game(){
    this.inGame = false;
    this.amountOfErrors = 0;
    this.goodAnswers = 0;
    this.time = 60;

    this.words = ["value" , "meanwhile", "finally" , "base" , "spikes" ,
                    "creature", "comforts" , "advanced" , "the snout" , "opposite",
                    "the spoon" , "the dust cloud" , "scrape" , "natural" , 
                    "significantly" , "the hedgehog" , "create" , "offset" , 
                    "enlarge" , "treacherous" , "the direction" , "the culture" , 
                    "the territory", "the timetable"];

    this.pickWord = function() {
        const randomIndex = Math.floor(Math.random() * this.words.length);
        return this.words[randomIndex];
    };

    ...
}

The game object would now be instantiated with:

let game = new Game();

While the whole code would now be longer, as you would have to manage both the page visual elements as well as the logic itself, it would also be more modular, organized and easier to maintain or add functionalities. It would still be necessary for some other function to call the game functions and update the corresponding page elements depending on the game state.

A more modern way of doing what i did above is using ES6 classes:

class Game {
    constructor() {
        this.inGame = false;
        this.amountOfErrors = 0;
        this.goodAnswers = 0;
        this.time = 60;

        this.words = ["value" , "meanwhile", "finally" , "base" , "spikes" ,
                    "creature", "comforts" , "advanced" , "the snout" , "opposite",
                    "the spoon" , "the dust cloud" , "scrape" , "natural" , 
                    "significantly" , "the hedgehog" , "create" , "offset" , 
                    "enlarge" , "treacherous" , "the direction" , "the culture" , 
                    "the territory", "the timetable"];
    }

    pickWord() {
        const randomIndex = Math.floor(Math.random() * this.words.length);
        return this.words[randomIndex];
    }

    ...
}

Answer 2

Code review

Deployment

I have included all the code in the snippet.

Yes, as far as I can tell. The best way to make sure is to run the code and test all parts of it.

The way of OOP

Is it smart to use OOP for a project like this?

Yes always smart, but then all JavaScript is OO so a little hard to avoid OOP when writing JavaScript.

The thing you need to ask is "Am I being smart and using good OOP practices" for which the answer is. You need to start creating objects that define and encapsulate the various parts of your application. Currently you are only using objects and indirectly creating them (functions are objects)

JavaScript is very flexible in this regard, there are many ways to create and use objects.

This type of application does not need to be defined as an instanceable object, as you will never need to create more than one copy of the game per page. In the rewrite I wrap all the code in the onload event function. This encapsulates all the data and functions, effectively making it a self contained anonymous object.

I have also added an object at the top to hold all the settings. This makes it easy to make changes, as you do not need to go through the code to change each setting (particularly handy when the code starts to run into 1000's of lines)

Some notes

• Always use strict mode by adding "use strict"; at the top of the JavaScript.

• Learn to use requestAnimationFrame to handle all DOM content changes rather than use time events. See the rewrite on how I use it.

• Forget that JavaScript has the == and != operators. You really never have to use them. The reason not to use them is subtle and full of coding bugs and traps for the unwary. Use the === and !== operators. To know more use google.

• Avoid setting element styles directly. Use the element's className or classList and set the class to change the style. See rewrite for example.

• Always put spaces between operators. Eg var sum=a+b+c; should be var sum = a + b + c;

• Use const for variables that do not change. EG var enteredWord = $("#wordField").val(); should be const enteredWord = $("#wordField").val();

• Math.random() will never return 1. When using random to pick an item from an array you DO NOT add one to the length of the array. See the snippet just below on how to pick a random item from an array.

  See the example for an alternative that removes the word from the woorden array, replacing it after the next word has been selected. This ensures you never get the same word twice and you don`t need to use the hacky recursive check.

const array = ["one","two","three"];
var randomItem = array[Math.random() * array.length | 0];  

// Note that |0 is a bitwise OR and a short cut for floor when flooring 
// positive numbers lower than 2 ** 31
var randomItem = array[Math.floor(Math.random() * array.length)]; 

• Don't clutter your code with noise. Comments should provide information that can not be deduced from the code.

  In the following you have 6 comments. Ask your self do you need to read them to know what the code is doing. If the answer is NO then they are noise and should not be there.

   function enterWord () {
     //If player is not in game
     if (inGame == false) {
       //Do nothing
     } else if (inGame == true) { //Is in game

       checkWord();
     }    }

   function checkWord () {
       var enteredWord = $("#wordField").val();

       //Check if the player typed the correct string
       if (enteredWord == currentWord) {
         //if true
         fadeColor("wordField", "backgroundColor", "rgb(43,234,81)");
         $("#wordField").val("");
         pickWord();
         goodAnswers++;
         previousWord = currentWord;

       } else {
         //if false
         amountOfErrors++;
         fadeColor("wordField", "backgroundColor", "rgb(234,43,43)");
       }   
  }

• You should not be querying the DOM for the same elements over and over. A DOM query is CPU intensive and pointless if you do it for the same item over and over.

  When your code starts do all the queries and store the elements for later use.

const wordField = document.querySelector("#wordField");
const currentWord = document.querySelector(("#currentWord");
const errors = document.querySelector("#errors");
const wpm = document.querySelector("#wpm");
const timer = document.querySelector("#timer");
const startBtn = document.querySelector("#startBtn");

Or use direct DOM refernces to access the DOM. Just use the elements id as a variable name. Be sure that element ID's are unique on the page.

Example of direct element reference, and what not to do.

   <!-- the html -->
   <div id = "myElementId">Content</div>

   <!-- the following is malformed HTML as the ids are not unique
        NEVER DO THIS!
   -->
   <div id = "badId">bad</div>
   <div id = "badId">bad</div>

   <!-- the following is malformed HTML as the id and name  are not unique 
        NEVER DO THIS!
   -->

   <div id = "inputID"> <input name ="inputId></input></div>


   <script>
        // direct access to the object via its id
        myElementId.textContent = "Hi there :)";


        // direct access to the non unique badId is now more complicated 
        // and browser dependent
        badId.addEventListeren(); // will throw an error as 
   </script>

JQuery

JQuery is to provide legacy browser support, its relevance is more and more questionable.

Do you really need jQuery? Most of what you use it for can be done using the native API.

The most negative down side to using JQuery is that you do not get practice using the DOM's native API's. You can see in the example that the code can be greatly simplified by not using jQuery and jQuery skills will not be in demand for much longer, while native DOM knowledge is always of value.

Rewrite

The rewrite is completely native, no additional javascript libraries are needed.

I have added three CSS rules using the classes .hide, .wordError, .wordCorrect

All element access is via direct reference using the element unique id. Note that the element id="currentWord" I changed to id = "currentWordElement"to prevent it clashing with thecurrentWord` variable defined in the code.

"use strict";
addEventListener("load", () => {

    var inGame = false;
    var currentWord, amountOfErrors, goodAnswers, endTime;

    document.addEventListener("keyup", (event) => {;
        if (event.code === "Enter") { checkWord() } 
    });
    startBtn.addEventListener("click", startGame );    
   
    const settings = {
        woorden : "value,meanwhile,finally,base,spikes,creature,comforts,advanced,the snout,opposite,the spoon,the dust cloud,scrape,natural,significantly,the hedgehog,create,offset,enlarge,treacherous,the direction,the culture,the territory,the timetable".split(","),
        errorClass : "wordError",
        correctClass : "wordCorrect",
        gameTime : 60 * 1000, // time in ms
        text : {
            complete : "Done...",
            errors : "Amount of errors: ",
            wpm : "Words per minute: ",   
        } ,           
    }


    function show(element) { element.classList.remove("hide") }

    function hide(element) { element.classList.add("hide") }

    function flashColour(type) {
        wordField.classList.add(settings[type + "Class"]);
        setTimeout(() => wordField.classList.remove(settings[type + "Class"]), 300);
    }

    function clearWord() {
        wordField.value = "";
        currentWordElement.textContent = "";
        if (currentWord) { settings.woorden.push(currentWord) }
        currentWord = undefined;
    }

    function pickWord () {
        const prevWord = currentWord;
        currentWord = settings.woorden.splice(Math.random() * settings.woorden.length | 0, 1)[0];
        wordField.value = "";
        currentWordElement.textContent = currentWord;
        if (prevWord) { settings.woorden.push(prevWord) }
    }

    function startGame() {
        endTime = undefined;
        inGame = true;
        wordField.focus();
        goodAnswers = amountOfErrors = 0;
        wpm.textContent = errors.textContent = currentWordElement.textContent = "";
        pickWord();
        show(timer);
        hide(startBtn);
        displayTimer();

    }

    function stopGame () {
        inGame = false;
        hide(timer)
        show(startBtn);
        clearWord();
        currentWordElement.textContent = settings.text.complete;
        errors.textContent = settings.text.errors + amountOfErrors;
        wpm.textContent = settings.text.wpm + goodAnswers;
    }

    function displayTimer() {
        if (endTime === undefined) { endTime = performance.now() + settings.gameTime }
        const remainingTime = endTime - performance.now();
        if (remainingTime <= 0) {
            timer.textContent = "";
            stopGame();
            return;
        }
        timer.textContent = (remainingTime / 1000).toFixed(2);
        requestAnimationFrame(displayTimer);
    }

    function checkWord () {
        if (inGame) {
            if (wordField.value === currentWord) {
                flashColour("correct");
                pickWord();
                goodAnswers ++;
            } else {
                amountOfErrors ++;
                flashColour("error");
            }
        }
    }
});

@import url('https://fonts.googleapis.com/css?family=IBM+Plex+Mono');

body {
  margin: 0;
  background-color: rgb(234,43,43);
  color: white;
  font-family: IBM Plex Mono;
}

.centerDiv {
  display: flex;
  flex-direction: column;
  margin: auto;
  width: 100%;
  height: 100%;
  align-items: center;
  text-align: center;
}

.headerDiv {
  display: flex;
  flex-direction: column;
  margin: auto;
  width: 100%;
  height: 100%;
  align-items: center;
  text-align: center;
  margin: 10px;
  margin-bottom: 10%;
}

.header {
  font-size: 25px;
}

.textfield {
  padding: 10px;
  border-radius: 3px;
  background-color: white;
  color: black;
  text-decoration: none;
  border: none;
  margin: 10px;
  font-size: 25px;
  transition: all 0.15s ease;
}

.textToType {
  font-size: 25px;
  margin: 10px;
}

.button {
  padding: 10px;
  border-radius: 3px;
  background-color: white;
  color: rgb(234,43,43);
  text-decoration: none;
  border: none;
  margin: 10px;
  font-size: 25px;
  transition: all 0.15s ease;
}
.button:hover {
  cursor: pointer;
  transform: scale(1.05);
}
.hide {
   display : none;
}
.wordError {
   background : rgb(234,43,43);
}

.wordCorrect {
   background : rgb(43,234,81);
}

<div class="centerDiv">

  <div class="headerDiv">
    <a class="header">Typing King</a>
    <a>Are you the next typing king?</a>
  </div>

  <div class="centerDiv">
    <a class="textToType" id="currentWordElement"></a>
    <input class="textfield" id="wordField" placeholder="Your input"/>
    <a class="button" id="startBtn">Start</a>
    <a id="timer"></a>
    <a id="wpm"></a>
    <a id="corrections"></a>
    <a id="errors"></a>
  </div>

</div>