JavaScript typing speed calculator

Question

I would like to kindly ask to review my code. I'd like suggestions, criticisms, and discussions on what is good and what could be done better.

const wordsArray = [ "aardvark", "albatross", "alligator", "alpaca", "ant", "anteater", "antelope", "ape", "armadillo", "donkey", "baboon", "badger", "barracuda", "bat", "bear", "beaver", "bee", "bison", "boar", "buffalo", "butterfly", "camel", "capybara", "caribou", "cassowary", "cat", "caterpillar", "cattle", "chamois", "cheetah", "chicken", "chimpanzee", "chinchilla", "chough", "clam", "cobra", "cockroach", "cod", "cormorant", "coyote", "crab", "crane", "crocodile", "crow", "curlew", "deer", "dinosaur", "dog", "dogfish", "dolphin", "dotterel", "dove", "dragonfly", "duck", "dugong", "dunlin", "eagle", "echidna", "eel", "eland", "elephant", "elk", "emu", "falcon", "ferret", "finch", "fish", "flamingo", "fly", "fox", "frog", "gaur", "gazelle", "gerbil", "giraffe", "gnat", "gnu", "goat", "goldfinch", "goldfish", "goose", "gorilla", "goshawk", "grasshopper", "grouse", "guanaco", "gull", "hamster", "hare", "hawk", "hedgehog", "heron", "herring", "hippopotamus", "hornet", "horse", "human", "hummingbird", "hyena", "ibex", "ibis", "jackal", "jaguar", "jay", "jellyfish", "kangaroo", "kingfisher", "koala", "kookabura", "kouprey", "kudu", "lapwing", "lark", "lemur", "leopard", "lion", "llama", "lobster", "locust", "loris", "louse", "lyrebird", "magpie", "mallard", "manatee", "mandrill", "mantis", "marten", "meerkat", "mink", "mole", "mongoose", "monkey", "moose", "mosquito", "mouse", "mule", "narwhal", "newt", "nightingale", "octopus", "okapi", "opossum", "oryx", "ostrich", "otter", "owl", "oyster", "panther", "parrot", "partridge", "peafowl", "pelican", "penguin", "pheasant", "pig", "pigeon", "pony", "porcupine", "porpoise", "quail", "quelea", "quetzal", "rabbit", "raccoon", "rail", "ram", "rat", "raven", "reindeer", "rhinoceros", "rook", "salamander", "salmon", "sandpiper", "sardine", "scorpion", "seahorse", "seal", "shark", "sheep", "shrew", "skunk", "snail", "snake", "sparrow", "spider", "spoonbill", "squid", "squirrel", "starling", "stingray", "stinkbug", "stork", "swallow", "swan", "tapir", "tarsier", "termite", "tiger", "toad", "trout", "turkey", "turtle", "viper", "vulture", "wallaby", "walrus", "wasp", "weasel", "whale", "wildcat", "wolf", "wolverine", "wombat", "woodcock", "woodpecker", "worm", "wren", "yak", "zebra" ];

const inputs = document.querySelector('.input');
const wordsContainer = document.querySelector('.wordsContainer')
let clickSum = 0;
let wordsSum = 0;
let currentWord = 0;
let timeStart = true;
const time = 30; // seconds

function displayWords(array) {
  wordsContainer.innerHTML = array.map( (value) => {return `<span class='word'>${value}</span>`}).join('');
}

function addActive() {
  const spans = document.querySelectorAll('.word');
  spans.item(currentWord).classList.add('active');
}

function removeActive() {
  document.querySelector('.active').classList.remove('active');
}

function onFlyCheck() {
  const span = document.querySelector('.active');
  const wordSpell = span.textContent.split('');
  const inputSpell = this.value.split('');
  const output = wordSpell.map( (value) => `<span>${value}</span>`);

  if (inputSpell.length > wordSpell.length) return;

  inputSpell.map( (value, index) => {
    if (wordSpell[index] === value) output[index] = `<span class="correct">${wordSpell[index]}</span>`;
    else output[index] = `<span class="incorrect">${wordSpell[index]}</span>`
  });
  span.innerHTML = output.join('');
}

function commitWord(e) {
  if (e.keyCode === 32 || e.keyCode === 13) {
    checkWord(this.value.trim());
    this.value = "";
  } 
}

function checkWord(word) {
  const active = document.querySelector('.active');
  active.innerHTML = wordsArray[currentWord];
  
  if (word === wordsArray[currentWord]) {
    active.classList.add('correct');
    wordsSum++;
    clickSum += (wordsArray[currentWord].length);
  } else {
    active.classList.add('incorrect')
  }

  currentWord++;
  removeActive();
  addActive();
  scrollWords();
}

function startTime() {
  if (timeStart) countDown();
  timeStart = false
}

function countDown() {
  setTimeout( endGame, time*1000);
}


function endGame() {
  const cpm = clickSum / time * 60;
  const score = document.querySelector('.score');
  inputs.disabled = true;

  score.innerHTML = `Congratulations! Your score is ... <br>
    <p>${cpm} key press per minute </p>
    <p>${wordsSum} correct word(s). </p> 
    <p> Refresh page (F5) to start again </p>`
}

function shuffle(array) {
  for (let i = array.length; i; i--) {
    let j = Math.floor(Math.random()*i);
    [array[i-1], array[j]] = [array[j], array[i-1]];
  }
}

let offset = 0;

function scrollWords() {
  const active = document.querySelector('.active');

  if (active.offsetTop -offset > active.offsetHeight) {
    offset += active.offsetHeight;
    wordsContainer.style.top =`-${offset}px`;
  }
}

inputs.addEventListener('keyup', onFlyCheck);
inputs.addEventListener('keyup', commitWord);
inputs.addEventListener('keypress', startTime);

shuffle(wordsArray);

displayWords(wordsArray);
addActive();

<!DOCTYPE html>
<html lang="en">
<head>
  <meta charset="UTF-8">
  <title>Type speed calculator</title>
</head>
<body>
  <div class="wrapper">
    <div class="container">
      <div class="wordsContainer">Loading sentence...</div>
    </div>
    <input type="text" class="input">
    <div class="score"></div>
  </div>
  
  <script type="text/javascript" src="scripts.js"></script>
</body>

<style>
  body {
    background-color: #ff6c00;
    font-size: 2rem;
  }
  .wrapper{
    width: 50%;
    margin: 75px auto;
    display: block;
  }
  .wordsContainer{
    position: relative;
    transition: all 0.2s; 
  }
  .container{
    text-align: center;
     /* padding: 1rem 2rem 0;  */
    width:100%;
    height: 10rem;
    overflow: hidden;
    margin: 1rem 2rem;
    display: inline-block;
    background-color: #ffc500;
    border-radius: 10px;
  }
  input{
    margin: 0;
    outline: none;
    width: 100%;
    font-size: 1.5rem;
    text-align: center;
    border-radius: 5px;
    border: 1px solid black;
  }
  .currentWord{
    background-color: white;
  }
  .word{
    display: inline-block;
    padding: 0.5rem 1rem;
  }
  .active{
    background-color: white;
    border-radius: .25em;
  }
  .incorrect{
    color: red;
  }
  .correct{
    color: green;
  }
  .score{
    margin: 125px auto;
    text-align: center;
  }
</style>
</html>

BenC · Accepted Answer · 2017-08-29 08:08:51Z

forEach their own

  inputSpell.map( (value, index) => {
    if (wordSpell[index] === value) output[index] = `<span class="correct">${wordSpell[index]}</span>`;
    else output[index] = `<span class="incorrect">${wordSpell[index]}</span>`
  });

Avoid using Array#map for side effects. If you don't care about the return value, use Array#forEach instead; it's more clear to the reader, and doesn't build up a temporary results array.

Minimizing the scope of the conditional could also be nice, depending on your taste:

inputSpell.forEach((value, i) => {
  const word = wordSpell[i];
  output[i] = `<span class="${word === value ? 'correct' : 'incorrect'}">${word}</span>`
});

index is a fine name; I've used i here since the function is so small and it's also a standard name.

Remove the magic

function commitWord(e) {
  if (e.keyCode === 32 || e.keyCode === 13) {

32 and 13 are "magic numbers"; factoring them out into named constants would be good. Plenty of people know that 13 is the enter key, but e.keyCode === KEYS.Enter requires no thought at all.

Also, .active is a class name that appears a handful of times. Once you see it two or three times, it's a good candidate for moving into a variable -- that prevents you from misspelling it and makes it easier to change in the future.

Implicits and organization

function displayWords(array) {
  wordsContainer.innerHTML = array.map( (value) => {return `<span class='word'>${value}</span>`}).join('');
}

Elsewhere you've used the implicit-return syntax, and it works here too. A more descriptive name than array would also be good.

function displayWords(words) {
  wordsContainer.innerHTML = words.map(word => `<span class="word">${word}</span>`).join('');
}

let offset = 0;

This is adrift near the bottom between a bunch of function declarations. Ideally it would go up top with the rest of the variables.

To jump into the HTML for a second, this:

<script type="text/javascript" src="scripts.js"></script>

can safely drop the type attribute; it's assumed by essentially every browser.

Also, the <style> should go in the <head>, either directly as-is or via a <link rel="stylesheet">.

Cohesion

A class may help organize your state more clearly.

Switching over would require

prefixing most things with this
changing some selections on document to be on the wordsContainer (optional, but in the spirit of reusability)

but it would provide some good encapsulation, more clearly showing what is/affects the game state and what is just utility methods. It would also likely be nicer to work with as you grow more features.

Here's a partial translation to a class approach for the sake of discussion:

const KEYS = Object.freeze({
  Enter: 13,
  Space: 32,
});

function shuffle(array) {
  for (let i = array.length; i; i--) {
    let j = Math.floor(Math.random()*i);
    [array[i-1], array[j]] = [array[j], array[i-1]];
  }
}

class TypingSpeed {
  constructor({ inputSelector, containerSelector }) { 
    this.$input = document.querySelector(inputSelector);
    this.$container = document.querySelector(containerSelector);
    this.wordClass = 'word';
    this.words = TypingSpeed.getWords();
    this.offset = 0;

    this.$input.addEventListener('keyup', this.onFlyCheck.bind(this));
    this.$input.addEventListener('keyup', thus.commitWord.bind(this));
    this.$input.addEventListener('keypress', this.startTime.bind(this));

    shuffle(this.words);

    this.displayWords(this.words);
    this.addActive();
  }

  // Pulled into a method to (a) clear up the constructor
  // and (b) you may want to eg. load from a json file
  static getWords() {
    return [.......];
  }

  displayWords(words) {
    this.$container.innerHTML = words.map(word => `<span class="${this.wordClass}">${word}</span>`).join('');
  }

  addActive() {
    const spans = this.$container.querySelectorAll(`${this.wordClass}`);
    spans.item(this.currentWord).classList.add('active');
  }

  ...
}

Really this looks nice in general. React and friends have their place, but it's fun to see straightforward use of querySelector, template string HTML, etc in a lightweight project like this.

Thank you BenC for your detail feedback. Class approach is not clear to me, could you advise any good materials? — korczas
– korczas, Commented Aug 29, 2017 at 14:59

Stack Exchange Network

JavaScript typing speed calculator

1 Answer 1

forEach their own

Remove the magic

Implicits and organization

Cohesion

You must log in to answer this question.

Hot Network Questions

JavaScript typing speed calculator

1 Answer 1

forEach their own

Remove the magic

Implicits and organization

Cohesion

You must log in to answer this question.

Related

Hot Network Questions