Typing game in JavaScript

Question

I recently made a typing game in JavaScript and I would like it to be reviewed so I can become better knowing where I made mistakes and what I should improve on.

CodePen

var temp = document.querySelector('.time');
 	var button = document.querySelector("button");
 	var words = document.querySelector(".words");
 	var timerDiv = document.querySelector(".time");
 	var scoreDiv = document.querySelector(".score");
 	var points = 0;
 	var spans;
 	var typed;
 	var seconds = 60;
 	var spark = new Audio("spark.mp3");

 	function countdown() {
 		points = 0;
 		var timer = setInterval(function(){
 			button.disabled = true;
    		seconds--;
    		temp.innerHTML = seconds;
    		if (seconds === 0) {
    			alert("Game over! Your score is " + points);
    			scoreDiv.innerHTML = "0";
    			words.innerHTML = "";
    			button.disabled = false;
    			clearInterval(timer);
    			seconds = 60;
    			timerDiv.innerHTML = "60";
    			button.disabled = false;	
    		}
 		}, 1000);
  }

  	function random() {
  		words.innerHTML = "";
  		var random = Math.floor(Math.random() * (1943 - 0 + 1)) + 0; // random number of 1943 words in the list
  		var wordArray = list[random].split("");
  		for (var i = 0; i < wordArray.length; i++) { //building the words with spans around the letters
  			var span = document.createElement("span");
  			span.classList.add("span");
  			span.innerHTML = wordArray[i];
  			words.appendChild(span);
  		}
  		spans = document.querySelectorAll(".span");
  	}

const list = [HERE IS A LIST OF 1900+ WORDS, so i cut it to save the space];


button.addEventListener("click", function(e){
  		countdown();
  		random();
  		button.disabled = true;	
  	});


  	function typing(e) {
  			typed = String.fromCharCode(e.which);
  			for (var i = 0; i < spans.length; i++) {
  				if (spans[i].innerHTML === typed) { // if typed letter is the one from the word
  					if (spans[i].classList.contains("bg")) { // if it already has class with the bacground color then check the next one
  						continue;
  					} else if (spans[i].classList.contains("bg") === false && spans[i-1] === undefined || spans[i-1].classList.contains("bg") !== false ) { // if it dont have class, if it is not first letter or if the letter before it dont have class (this is done to avoid marking the letters who are not in order for being checked, for example if you have two "A"s so to avoid marking both of them if the first one is at the index 0 and second at index 5 for example)
              spans[i].classList.add("bg");
  						break;
  					}
  				}
  			}
  			var checker = 0;
  			for (var j = 0; j < spans.length; j++) { //checking if all the letters are typed
  				if (spans[j].className === "span bg") {
  					checker++;
  				}
  				if (checker === spans.length) { // if so, animate the words with animate.css class
  					spark.pause();
					  spark.currentTime = 0;
  					spark.play();
  					words.classList.add("animated");
  					words.classList.add("fadeOut");
  					points++; // increment the points
  					scoreDiv.innerHTML = points; //add points to the points div
  					document.removeEventListener("keydown", typing, false);
  					setTimeout(function(){
  						words.className = "words"; // restart the classes
  						random(); // give another word
  						document.addEventListener("keydown", typing, false);
  					}, 400);
  				}

  			}
  	}

    button.addEventListener("click", function(e){
      countdown();
      random();
      button.disabled = true; 
      document.addEventListener("keydown", typing, false);
    });

* {
	margin: 0;
	padding: 0;
	box-sizing: border-box;
	-webkit-box-sizing: border-box;
}

body {
	background-color: #353535;
	font-family: 'Raleway', sans-serif;
}

.wrapper {
	max-width: 600px;
	margin: 0 auto;
	width: 100%;
	text-align: center;
	padding: 2%;
	background-color: #424242;
	height: 500px;
}

h1 {
    color: #ecf0f1;
}

h1 + p {
	margin-bottom: 5%;
    color: #3498db;
}

.scoreWrap {float: left;}
.timeWrap {float: right;}

.outerWrap:after {
	content: "";
	display: block;
	clear: both;
}

.bg {
	background-color: #04AF71;
}

button {
   border: none;
  background-color: #FF7373;
  box-shadow: 0px 5px 0px 0px #CE4646;
  outline: none;
  border-radius: 5px;
  padding: 10px 15px;
  font-size: 22px;
  text-decoration: none;
  margin: 20px;
  color: #fff;
  position: relative;
  display: inline-block;
  cursor: pointer;
}

button:active {
  transform: translate(0px, 5px);
  -webkit-transform: translate(0px, 5px);
  box-shadow: 0px 1px 0px 0px;
}

.scoreWrap p, .scoreWrap span, .timeWrap p, .timeWrap span {
    font-size: 30px;
    color: #FF7373;
}

.wordsWrap {
	margin-top: 50px;
}

.words span{
    font-size: 60px;
    letter-spacing: 1px;
    color: #ECF0F1;
}

<html>
<head>
	<meta charset="utf-8">
	<meta http-equiv="X-UA-Compatible" content="IE=edge">
	<title>Typing game</title>
	<link href='https://fonts.googleapis.com/css?family=Raleway:400,700' rel='stylesheet' type='text/css'>
	<link rel="stylesheet" href="animate.css">
	<link rel="stylesheet" href="main.css">
</head>
<body>
	<div class="wrapper">
		<h1>Typing game by Nikola Simovic</h1>
		<p>Type as many words as you can until time runs out!</p>
		<button>START</button>
		<div class="outerWrap">
			<div class="scoreWrap">
				<p>Score</p>
				<span class="score">0</span>
			</div>
			<div class="timeWrap">
				<p>Time left</p>
				<span class="time">60</span>
			</div>
		</div>
		<div class="wordsWrap">
			<p class="words"></p>
		</div>
	</div>

Answer 1

After taking a look at your code, I have a few generic suggestions:

First of all, and this applies to JS/HTML/CSS, you have to work on a better indentation. 8 spaces seems too much. In most cases you are not consistent with this.

Choose either 2 or 4 spaces and stick to that. Once you have rules your code will be easier to read. Remember that code is read more times than is written.

For CSS, have your class/id declaration in the first line with an open brace, and always start with the properties on a new line, avoiding one line stuff like .scoreWrap {float: left;}.

Still in CSS, go from more generic to more specific. You started well with properties for *, then body. It would be nice to continue with h1, then button, then button:active, and later classes / identifiers. That way is easier to locate a specific class.

For the JavaScript part, I'll try to spend more time on the naming of functions and objects. It's difficult to read a function named 'typing' and be sure what is being done there. Same goes for 'countdown' or 'random'.

Once you try to give better names to those functions, you'll realize maybe they are doing a bit more that they are intending to do, and start dividing the code in smaller pieces, with proper naming

The typing function is also quite big. I recommend dividing it into smaller functions with a proper naming too.

Answer 2

How and Why to Avoid setInterval

In general, it's best to avoid setInterval() if you can help it. The primary reason is that setInterval() has the potential to keep issuing more calls to the specified function while the function code itself is being executed during a preceding execution.

An extreme example would be a case where the specified function takes a full second to run, but the interval is set to execute every 100 milliseconds; in such a case, by the time the first execution is complete, ten further calls would waiting execution.

This isn't an obvious problem when you're working with large enough intervals and synchronous code, but long-running functions (especially asynchronous or looping functions) could lead to unexpected behavior, so it's best to break out of the setInterval() habit before it bites you.

A decent alternative to setInterval() is to use setTimeout(). Within the function passed to setTimeout(), you can decide whether or not to repeat; if you want to repeat, just call setTimeout() again.

Here's an amended version of your countdown function, with an additional update function to be used in the setTimeout() call.

function countdown(){
    points = 0;
    setTimeout(update,1000);
}
function update(){
    button.disabled = true;
    seconds--;
    temp.innerHTML = seconds;
    if (seconds === 0) {
        alert("Game over! Your score is " + points);
        scoreDiv.innerHTML = "0";
        words.innerHTML = "";
        button.disabled = false;
        seconds = 60;
        timerDiv.innerHTML = "60";
        button.disabled = false;    
    }else{
        setTimeout(update,1000);
}

Removing Unnecessary Math

var random = Math.floor(Math.random() * (1943 - 0 + 1)) + 0;

Unless I'm missing something obscure here, that line seems equivalent to the following:

var random = Math.floor(Math.random() * 1944);

Efficient Looping in JavaScript

Many browsers will see a performance improvement if you make a slight modification to your for loops.

Take a look at your for loop in the random() function: for (var i = 0; i < wordArray.length; i++)

That loop requires that the browser check the length of the array after every iteration. Since you know the array's length isn't changing, you can cache that length at the same time that you initialize your iteration variable.

for (var i = 0, len = wordArray.length; i < len; i++)

For some browsers, the performance impact is significant. For reference, here's a relevant JS perf test: http://jsperf.com/thrig-for-loop-array-access

Avoiding Unnecessary DOM Queries

You did a good job by caching (that is, saving to variables) most your query selections up front. This is much better than querying the DOM on the fly every time you want to access those elements.

There's only one spot where it looks like you're querying the DOM unnecessarily, inside your random function:

for (var i = 0; i < wordArray.length; i++) { 
    var span = document.createElement("span");
    span.classList.add("span");
    span.innerHTML = wordArray[i];
    words.appendChild(span);
}
spans = document.querySelectorAll(".span");

Since you're already looping through the elements in the wordArray and creating each span element one at a time, you might as well just tuck away a reference to each span as you create it.

spans = [];
for (var i = 0, len = wordArray.length; i < len; i++) { 
    var span = document.createElement("span");
    span.classList.add("span");
    span.innerHTML = wordArray[i];
    words.appendChild(span);
    spans.push(span);
}

Consider More Specific Function Names

I can see what you were going for with countdown(), but random() and typing() are a bit abstract.

In general, try to make your function names into verbs or verb phrases that indicate what the functions do.

I'd suggest renaming random to something like getNewWord, and typing to something like checkKey.

The extra effort of going back and renaming your functions pays off in the long run by making it easier for others (and your future self) to understand what the code is doing at a glance.