1
\$\begingroup\$

I created a typing test app. I would love some feedback from you guys about my code so I can improve.
Here is github repo and hosted preview.

'use strict';

const block = document.querySelector('.words');
const inputBox = document.querySelector('.input');

let wordIds;
let wordsBlock = [];
let currentWord;
let wpm = 0;
let accuracy = 0;
const wordsInBlock = 25;

const keywords = [
  'ability',
  'able',
  'about',
  'above',
  'accept',
  'according',
  'account',
  'across',
  'act',
  'action',
  'activity',
  'actually',
  'behavior',
  'behind',
  'believe',
  'benefit',
  'best',
  'better',
  'between',
  'beyond',
  'big',
  'bill',
  'billion',
  'bit',
  'black',
  'blood',
  'blue',
  'board',
  'body',
  'book',
  'born',
  'both',
  'box',
  'boy',
  'break',
  'case',
  'catch',
  'cause',
  'cell',
  'center',
  'central',
  'century',
  'certain',
  'certainly',
  'chair',
  'challenge',
  'chance',
  'change',
  'character',
  'charge',
  'check',
  'child',
  'choice',
  'choose',
  'church',
  'citizen',
  'city',
  'civil',
  'claim',
  'class',
  'clear',
  'clearly',
  'close',
  'coach',
  'cold',
  'collection',
  'college',
  'color',
  'come',
  'commercial',
  'common',
  'community',
  'eight',
  'either',
  'election',
  'else',
  'employee',
  'end',
  'energy',
  'enjoy',
  'enough',
  'enter',
  'entire',
  'environment',
  'environmental',
  'especially',
  'establish',
  'even',
  'evening',
  'event',
  'ever',
  'every',
  'everybody',
  'everyone',
  'everything',
  'evidence',
  'exactly',
  'example',
  'executive',
  'exist',
  'expect',
  'experience',
  'expert',
  'explain',
  'eye',
  'face',
  'fact',
  'factor',
  'fail',
  'fall',
  'family',
  'far',
  'fast',
  'father',
  'fear',
  'federal',
  'feel',
  'feeling',
  'few',
  'field',
  'fight',
  'figure',
  'fill',
  'film',
  'final',
  'finally',
  'financial',
  'find',
  'fine',
  'finger',
  'finish',
  'fire',
  'firm',
  'first',
  'fish',
  'five',
  'floor',
  'fly',
  'focus',
  'follow',
  'food',
  'foot',
  'for',
  'force',
  'foreign',
  'forget',
  'form',
  'former',
  'forward',
  'four',
  'free',
  'friend',
  'from',
  'front',
  'full',
  'fund',
  'future',
  'game',
  'garden',
  'gas',
  'general',
  'generation',
  'get',
  'girl',
  'give',
  'glass',
  'go',
  'goal',
  'good',
  'government',
  'great',
  'green',
  'ground',
  'group',
  'grow',
  'growth',
  'guess',
  'gun',
  'guy',
  'hair',
];

// ######################################################################
// Generate keywords

const generateOutput = () => {
  let randomNumber;
  for (let i = 0; i < wordsInBlock; i++) {
    randomNumber = Math.floor(Math.random() * keywords.length);
    wordsBlock.push(keywords[randomNumber]);
    block.innerHTML += `<p class='word-ids' id='word-id${i}'>${keywords[randomNumber]}</p>`;
  }
  wordIds = document.querySelectorAll('.word-ids');
  currentWord = 0;
  wordIds[currentWord].classList.toggle('highlight');
};

generateOutput();

// ######################################################################
// Reset

const reset = (type) => {
  if (type === 'complete') {
    wpm = 1;
    accuracy = 1;
    secondsLeft = 59;
    inputBox.disabled = false;
    clearInterval(timer);
    results.classList.add('hidden');
  }
  block.textContent = '';
  wordsBlock = [];
  inputBox.value = '';
  generateOutput();
};

// ######################################################################
// Game

inputBox.addEventListener('keyup', (event) => {
  if (event.code === 'Space') {
    if (secondsLeft === 59) setInterval(timer, 1000);
    if (currentWord === wordsInBlock - 1) {
      reset();
    } else {
      if (inputBox.value.trim() === wordsBlock[currentWord]) {
        wordIds[currentWord].classList.toggle('mark-green');
        accuracy++;
      } else wordIds[currentWord].classList.toggle('mark-red');

      wordIds[currentWord].classList.toggle('highlight');
      currentWord++;
      wordIds[currentWord].classList.toggle('highlight');
      inputBox.value = '';
      wpm++;
    }
  }
});

// ######################################################################
// Restart

const restartBtn = document.querySelector('.restart');

restartBtn.addEventListener('click', () => {
  reset('complete');
});

// ######################################################################
// Timer

const timerBox = document.querySelector('.timer');

const results = document.querySelector('.results');
const wpmBox = document.querySelector('.wpm');
const accuracyBox = document.querySelector('.accuracy');

let interval;
let secondsLeft = 59;

const timer = () => {
  if (secondsLeft > 0) {
    if (secondsLeft >= 10) timerBox.textContent = `0:${secondsLeft}`;
    else timerBox.textContent = `0:0${secondsLeft}`;
    secondsLeft--;
  } else {
    clearInterval((secondsLeft = 0));
    wpmBox.textContent = wpm;
    accuracyBox.textContent = `${parseInt((accuracy / wpm) * 100)}%`;
    inputBox.disabled = true;
    results.classList.remove('hidden');
  }
};
@import url("https://fonts.googleapis.com/css2?family=Poppins:wght@300;500;600&display=swap");
*,
*::after,
*::before {
  padding: 0;
  margin: 0;
  -webkit-box-sizing: border-box;
          box-sizing: border-box;
  font-family: 'Poppins', sans-serif;
}

body {
  background-color: #f2bac9;
  height: 100vh;
  display: -webkit-box;
  display: -ms-flexbox;
  display: flex;
  -webkit-box-pack: center;
      -ms-flex-pack: center;
          justify-content: center;
  -webkit-box-align: center;
      -ms-flex-align: center;
          align-items: center;
}

.main {
  background-color: #e887a1;
  border-radius: 10px;
  padding: 5rem 10rem;
  text-align: center;
}

.main h2 {
  font-weight: 600;
  font-size: 2rem;
  color: white;
}

.main .words {
  border-radius: 3px;
  padding: 1rem 2rem;
  margin-top: 3rem;
  margin-bottom: 3.5rem;
  width: 50rem;
  overflow: hidden;
  background-color: white;
  color: black;
  display: -webkit-box;
  display: -ms-flexbox;
  display: flex;
  -ms-flex-wrap: wrap;
      flex-wrap: wrap;
  -webkit-box-pack: center;
      -ms-flex-pack: center;
          justify-content: center;
  -webkit-box-align: center;
      -ms-flex-align: center;
          align-items: center;
}

.main .words p:not(:first-child) {
  margin-left: 0.2rem;
}

.main .menu {
  display: -webkit-box;
  display: -ms-flexbox;
  display: flex;
}

.main .input {
  -webkit-box-flex: 1;
      -ms-flex: 1;
          flex: 1;
  padding: 1rem 1.5rem;
  border-radius: 3px;
  border: none;
  outline: none;
}

.main .timer,
.main .restart {
  padding: 1rem 1.5rem;
  border-radius: 3px;
  margin-left: 0.5rem;
}

.main .timer {
  background-color: #d90368;
  color: white;
}

.main .restart {
  background-color: #4ecdc4;
  display: -webkit-box;
  display: -ms-flexbox;
  display: flex;
  cursor: pointer;
}

.main .restart img {
  width: 1.5rem;
}

.results {
  background-color: white;
  border-radius: 10px;
  padding: 3rem 10rem;
  text-align: center;
  font-size: 1.6rem;
  display: -webkit-box;
  display: -ms-flexbox;
  display: flex;
  -webkit-box-pack: justify;
      -ms-flex-pack: justify;
          justify-content: space-between;
  font-weight: 300;
  -webkit-transition: all 0.5s;
  transition: all 0.5s;
}

.results span {
  font-weight: bold;
}

.highlight {
  background-color: #4ecdc4;
  padding: 0.1rem 0.2rem;
}

.mark-green {
  color: #02cc02;
}

.mark-red {
  color: red;
}

.hidden {
  visibility: hidden;
}
<!DOCTYPE html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <title>Typing test</title>
    <link rel="stylesheet" href="style/main.css">
</head>
<body>
    <main>
        <div class="main">
            <h2>Typing test</h2>
            <div class="words"></div>
            <div class="menu">
                <input type="text" class="input">
                <div class="timer">1:00</div>
                <div class="restart">
                    <img src="refresh.svg" alt="refresh">
                </div>
            </div>
        </div>
        <div class="results hidden">
            <p>wpm: <span class="wpm">88</span></p>
            <p>accuracy: <span class="accuracy">75%</span></p>
        </div>
    </main>
    <script src="script.js"></script>
</body>
</html>

\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

Here's a few thoughts

  • Put your list of words in its own file - a large chunk of data doesn't belong intermixed in the code, it gets in the way.
  • Avoid innerHTML whenever possible - it opens your app up for a lot of potential security exploits (e.g. XXS attacks). Just use the DOM manipulation functions like document.createElement(). If you find them hard to work with, consider making helper functions (this code-review answer has some nice suggestions)
  • I like extracting out little utility functions, like chooseRandomEntry(someList) -> randomElementFromList. This can help improve the readability of functions. i.e. if you used this helper in generateOutput(), then generateOutput can focus more on saying what needs to be done to actually generate the output (the purpose of the function), and doesn't have to spell the step by step mundane details of finding a random element in an array. It makes it much easier to understand what the function does at a glance.
  • Be very careful with your variable naming. Make sure the names accurately describe their purpose. Good naming really helps make a program readable. Bad naming causes readers to have to look at how a variable is being used before they know what it really means, which takes a lot more time. For example, in your code, wpm doesn't really mean words-per-minute, it actually means "how many words the user has typed in thus far" which you eventually use to calculate the wpm. Maybe a better name would be wordCount. Same goes for accuracy - it does not hold an accuracy ratio, rather, it holds the number of correct words the user has typed in. So maybe numberOfCorrectWords or correctWordCount (don't be afraid of longer names if it helps make the intent more clear).
  • This is more UX than code-review, but I noticed in the live preview, there's a lot of content shifting around. Give the timer box a fixed-width so it doesn't keep changing size as it counts down. Give each word enough padding so that when you put the highlighted box on one of them, you don't have to add more padding and shift the words around.

A bigger change that would really help improve this code would be to separate your "view" logic from your "model", or state. What this would look like is your user does some action, triggering an event that update the model (state), then informs the view of this update.

A rough example of one possible implementation of this:

// model (or application state)
const createInitialState = () => ({
  wpm: 1,
  accuracy: 1,
  secondsLeft: 59,
  timerRunning: false,
  wordsBlock: generateRandomWordList({size: wordsInBlock})
});

let state = createInitialState();

// Your event listener
const completeReset = () => {
  state = createInitialState();
  updateView();
};

// view
const updateView = () => {
  inputBox.disabled = !state.timerRunning;
  if (!state.timerRunning && currentInterval == null) {
    clearInterval(currentInterval);
    currentInterval = null;
  } else if (state.timerRunning && currentInterval != null) {
    currentInterval = setInterval(countDown, 1000);
  }
  // etc
}

That's just a quick and dirty example, don't take that specific example too seriously, you can figure out how to best make the general idea work in your program as you go along. But it should show how you might be able to segregate the view from the model. And notice how updateView() is able to figure itself out based on the model alone - this isn't always possible, but its good to do when it makes sense as it often simplifies logic. i.e. if everything is coded right, as soon as your timer reaches zero, just set timerRunning to false in the state and notify the view, and the view will update itself to render stats to the user.

\$\endgroup\$
1
  • \$\begingroup\$ thanks for the feedback \$\endgroup\$
    – fluffy
    Dec 19, 2020 at 23:59

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.