Simple math game in JavaScript

Question

Created a simple math (addition/subtraction) game to practice my JavaScript. Any feedback on the JavaScript appreciated.

Can play in: CodePen

Instructions: A math equation will show up and simply press the corresponding number on your keyboard to answer. Speed game so no need to press enter or anything, but also means no deleting/going back if press wrong key.

Note: The formula to create the equation could be easily change to make different levels. I made it simple to understand/program. This one allows for subtracting a negative number, which people have complained about, and I could easily prevent but I like the extra layer.

// set HTML shortcut variables
const startButton = document.getElementById('start');
const questionDiv = document.getElementById('question');
const score = document.getElementById('score');
const red = document.getElementById('red-light');
const green = document.getElementById('green-light');
const timer = document.getElementById('timer');
const slideDiv = document.getElementById('timer-slide');
const timerLabel = document.getElementById('timer-label');
// helper functions
function setHTML(el, text){
 el.innerHTML = text;
}
function light(light){
 window.clearTimeout(reset);
 red.style.background = "black";
 green.style.background = "black";
 if (light === "red") red.style.background = "white";
 if (light === "green") green.style.background = "white";
 reset = setTimeout(resetLights, 500);
}
let reset;
function resetLights(){
 light("");
 }
function slide(bool){
 if(bool){
 slideDiv.classList.add('slid-up');
 } else {
 slideDiv.classList.remove('slid-up');
 // slideDiv.style.webkitTransform = "translateY(0)";
 }
}
// listener functions
document.addEventListener('keypress', (event) => {
 const keyName = event.key;
 const number = parseInt(keyName);
 if (isNaN(number)) return;
 ob.enteredAnswer = number;
 ob.checkAnswer();
});
startButton.addEventListener('click', (event) => {
 ob.start();
});
// create Object
const ob = new Object;
ob.score = 0;
ob.inProgress = false;
ob.countDown = false;
ob.countDownStart = Date.now();
ob.startTime = Date.now();
// Object Methods
ob.start = function(){
 if (ob.inProgress) return;
 if (ob.countDown) return;
 setHTML(timerLabel, "Starting in");
 slide(false);
 ob.countDownStart = Date.now();
 ob.countDown = true;
}
ob.begin = function(){
 if (ob.inProgress) return;
 ob.score = 0;
 setHTML(score, ob.score);
 ob.countDown = false;
 ob.startTime = Date.now()
 ob.inProgress = true;
 ob.newQuestion();
}
ob.newQuestion = function(){
 const a = Math.floor(Math.random() * 10);
 const r1 = Math.floor(Math.random() * 10);
 const r2 = Math.floor(Math.random() * 10);
 const r3 = r1 + r2 - a;
 const string = `${r1} + ${r2} - ${r3}`;
 this.correctAnswer = a;
 setHTML(questionDiv, string);
}
ob.checkAnswer = function(){
 if (!ob.inProgress) return;
 if (ob.correctAnswer === ob.enteredAnswer){
 // setHTML(result, "Correct!");
 light("green");
 ob.scoreChange(100);
 } else {
 // setHTML(result, "Incorrect!");
 ob.scoreChange(-50);
 light("red");
 }
 ob.newQuestion();
}
ob.scoreChange = function(change){
 ob.score += change;
 setHTML(score, ob.score);
}
ob.end = function(){
 ob.inProgress = false;
 setHTML(questionDiv, "");
 slide(true);
}
// Timer functions
function checkTimers(){
 if (ob.countDown){
 const diff = 3 - Math.floor((Date.now() - ob.countDownStart) / 1000);
 if (diff > 0){
 setHTML(timer, diff);
 } else {
 ob.begin();
 }
 }
 if (ob.inProgress){
 const diff = 15 - Math.floor((Date.now() - ob.startTime) / 1000);
 if (diff > -1){
 setHTML(timerLabel, "Remaining");
 setHTML(timer, diff);
 } else {
 ob.end();
 }
}
}
setInterval(checkTimers, 100);

body{
 display: flex;
 flex-flow: column;
 justify-content: center;
 align-items: center;
 text-align: center;
 min-height: 70vh;
 background: #1d1f20;
 color:white;
}
div{
 display: flex;
 justify-content: center;
 align-items: center;
 flex-flow: column nowrap;
}
section{
 border: 2px solid white;
}
section > div{
 margin: 5px;
 height: 4em;
 width: 10em;
 border-radius: 5px;
}
.score-container{
 background: #568bbd;
}
div.problem{
 background: #86b38a;
}
p.main{
 font-size: 1.4rem;
 margin: 5px;
}
p.label{
 font-size: 0.8rem;
 margin: 2px;
}
.lights{
 flex-flow: row nowrap;
}
.light{
 height: 20px;
 width: 20px;
 border-radius: 50%;
 margin: 5px;
}
#green-light::after{
 content: '';
 width: 100%;
 height: 100%;
 background: #00ff00aa;
 border-radius: 50%;
 position: relative;
 z-index: 2;
}
#red-light::after{
 content: '';
 width: 100%;
 height: 100%;
 background: #ff0000aa;
 border-radius: 50%;
 position: relative;
 z-index: 2;
}
.white{
 background: white;
}
button{ 
 border: 2px solid #42964c;
 background-color: #86b38a;
 color: white;
 border-radius: 10px;
 padding: 0.5em;
 font-size: 1.4rem;
 cursor: pointer;
 box-shadow: 0 5px 0 #42964c;
 transition-duration: 0.2s;
 text-shadow: 2px 2px 0 #42964c;
 outline: 0;
 }
button:hover{ 
 transform: translateY(5px);
 box-shadow: 0 0 0 #42964c;
}
button:active{
 transform: scale(0.8);
}
.slide-container{
 position: relative;
 overflow: hidden;
}
.slide-cover{
 position: absolute;
 height: 100%;
 width: 100%;
 top: 0;
 left: 0;
 transition-duration: 0.2s;
 transform: translateY(0);
}
.slid-up{
 transform: translateY(-120%);
}
#timer-slide{
 background: #d0782a;
}

<section>
 <div class='start-container slide-container'>
 <button id="start">START</button>
 <div id="timer-slide" class='timer slide-cover slid-up'>
 <p id="timer-label" class='label'>Timer</p>
 <p id="timer" class='main'></p>
 </div>
</div>
<div class='problem'>
 <p class='label'>PROBLEM</p>
 <p id="question" class='main'></p>
</div>
<div class='lights'>
 <div id='green-light' class='light'></div>
 <div id='red-light' class='light'></div>
</div>
<div class='score-container'>
 <p class='label'>SCORE</p>
 <p id="score" class='main'></p>
</div>
</section>
<p class='label'>(Use Numbers on Keyboard)</p>

Feel free to fork and add to it!

• radarbob: Even in the answers the "better" variable names are bad. Naming is essential to understanding the domain and code structure. As code grows bad naming proliferates, obfuscating the code in a non-linear fashion. In any program, expecially production programs, thoughtless names like "object" or "op2 is better than r2" is the start of a maintenance death spiral. Hear me now and believe me later.

Answer 1

Feedback

I like how the DOM references are cached at the start. There is also good usage of const and let for constants and block-scoped variables. For a dynamic algebraic quiz it is a novel idea.

Suggestions

JavaScript

Variable names

Some variable names could be more appropraitely named - e.g. in ob.newQuestion(), the constants r1, r2 and r3 might be better named op1, op2 and op3 for the operands... Also, adding a constant for the answer a is unnecessary. One can simply assign the value to the property and utilize that in the rest of that method.

this.correctAnswer = Math.floor(Math.random() * 10);

And then string might be better named something like formula or equation. Also, the parameter name in slide() is pretty generic: bool. Perhaps up or isSlideUp would be a better name.

Default values

ecmascript-6 offers default values for parameters. This could be utilized in functions like slide:

function slide(up = false)

line endings after function definitions

Methods are defined on ob like ob.newQuestion = function() { ...} without a line ending. While line endings can be automatically inserted by the JS engine, it would be better to be consistent. And Such a function could also be expressed as an arrow function: ob.newQuestion = _ => { ...};

Simplify one-line lambdas

There is often little need to create a lamda/anonymous function just to call a function. For instance:

startButton.addEventListener('click', (event) => {
 ob.start();
 });

This can be simplified to the follow (presuming it is moved after the declaration of ob.start):

 startButton.addEventListener('click', ob.start);

Bear in mind that the event object would thus be passed as the start parameter to ob.start.

ParseInt without radix

I see one line that parses the response from the user:

 const number = parseInt(keyName);

But there is no radix passed. If you are going to use parseInt(), it is wise to specify the radix using the second parameter - unless you are using a unique number system like hexidecimal, octal, etc. then specify 10 for decimal numbers.

Always specify this parameter to eliminate reader confusion and to guarantee predictable behavior. Different implementations produce different results when a radix is not specified, usually defaulting the value to 10.¹

return parseInt(this.newSum[0], 10);

While your code specifically handles single digit numbers, it is a good habit to always pass that radix.

As recommended by MDN, pass the radix - presumably 10 for base-10 numbers.

 const number = parseInt(keyName, 10);

CSS

There is some redundancy that can be abstracted- e.g. both #green-light::after{ and #red-light::after{ have 6 out of 7 identical lines. Those can be pulled out into a class selector. That way if one of those styles needs to be updated, it can be done in one place instead of two:

.light::after {
 content: '';
 width: 100%;
 height: 100%;
 border-radius: 50%;
 position: relative;
 z-index: 2;
}
#green-light::after{
 background: #00ff00aa;
}
#red-light::after{
 background: #ff0000aa;
} 

Updated code

// set HTML shortcut variables
const startButton = document.getElementById('start');
const questionDiv = document.getElementById('question');
const score = document.getElementById('score');
const red = document.getElementById('red-light');
const green = document.getElementById('green-light');
const timer = document.getElementById('timer');
const slideDiv = document.getElementById('timer-slide');
const timerLabel = document.getElementById('timer-label');
// helper functions
function setHTML(el, text){
 el.innerHTML = text;
}
function light(light){
 window.clearTimeout(reset);
 red.style.background = "black";
 green.style.background = "black";
 if (light === "red") red.style.background = "white";
 if (light === "green") green.style.background = "white";
 reset = setTimeout(resetLights, 500);
}
let reset;
function resetLights(){
 light("");
 }
function slide(up = false){
 if(up){
 slideDiv.classList.add('slid-up');
 } else {
 slideDiv.classList.remove('slid-up');
 // slideDiv.style.webkitTransform = "translateY(0)";
 }
}
// listener functions
document.addEventListener('keypress', (event) => {
 const keyName = event.key;
 const number = parseInt(keyName, 10);
 if (isNaN(number)) return;
 ob.enteredAnswer = number;
 ob.checkAnswer();
});
// create Object
const ob = new Object;
ob.score = 0;
ob.inProgress = false;
ob.countDown = false;
ob.countDownStart = Date.now();
ob.startTime = Date.now();
// Object Methods
ob.start = function(){
 if (ob.inProgress) return;
 if (ob.countDown) return;
 setHTML(timerLabel, "Starting in");
 slide();
 ob.countDownStart = Date.now();
 ob.countDown = true;
}
startButton.addEventListener('click', ob.start);
ob.begin = function(){
 if (ob.inProgress) return;
 ob.score = 0;
 setHTML(score, ob.score);
 ob.countDown = false;
 ob.startTime = Date.now()
 ob.inProgress = true;
 ob.newQuestion();
}
ob.newQuestion = function(){
 this.correctAnswer = Math.floor(Math.random() * 10);
 const op1 = Math.floor(Math.random() * 10);
 const op2 = Math.floor(Math.random() * 10);
 const op3 = op1 + op2 - this.correctAnswer;
 const string = `${op1} + ${op2} - ${op3}`;
 setHTML(questionDiv, string);
}
ob.checkAnswer = function(){
 if (!ob.inProgress) return;
 if (ob.correctAnswer === ob.enteredAnswer){
 // setHTML(result, "Correct!");
 light("green");
 ob.scoreChange(100);
 } else {
 // setHTML(result, "Incorrect!");
 ob.scoreChange(-50);
 light("red");
 }
 ob.newQuestion();
}
ob.scoreChange = function(change){
 ob.score += change;
 setHTML(score, ob.score);
}
ob.end = function(){
 ob.inProgress = false;
 setHTML(questionDiv, "");
 slide(true);
}
// Timer functions
function checkTimers(){
 if (ob.countDown){
 const diff = 3 - Math.floor((Date.now() - ob.countDownStart) / 1000);
 if (diff > 0){
 setHTML(timer, diff);
 } else {
 ob.begin();
 }
 }
 if (ob.inProgress){
 const diff = 15 - Math.floor((Date.now() - ob.startTime) / 1000);
 if (diff > -1){
 setHTML(timerLabel, "Remaining");
 setHTML(timer, diff);
 } else {
 ob.end();
 }
}
}
setInterval(checkTimers, 100);

body{
 display: flex;
 flex-flow: column;
 justify-content: center;
 align-items: center;
 text-align: center;
 min-height: 70vh;
 background: #1d1f20;
 color:white;
}
div{
 display: flex;
 justify-content: center;
 align-items: center;
 flex-flow: column nowrap;
}
section{
 border: 2px solid white;
}
section > div{
 margin: 5px;
 height: 4em;
 width: 10em;
 border-radius: 5px;
}
.score-container{
 background: #568bbd;
}
div.problem{
 background: #86b38a;
}
p.main{
 font-size: 1.4rem;
 margin: 5px;
}
p.label{
 font-size: 0.8rem;
 margin: 2px;
}
.lights{
 flex-flow: row nowrap;
}
.light{
 height: 20px;
 width: 20px;
 border-radius: 50%;
 margin: 5px;
}
.light::after {
 content: '';
 width: 100%;
 height: 100%;
 border-radius: 50%;
 position: relative;
 z-index: 2;
}
#green-light::after{
 background: #00ff00aa;
}
#red-light::after{
 background: #ff0000aa;
}
.white{
 background: white;
}
button{ 
 border: 2px solid #42964c;
 background-color: #86b38a;
 color: white;
 border-radius: 10px;
 padding: 0.5em;
 font-size: 1.4rem;
 cursor: pointer;
 box-shadow: 0 5px 0 #42964c;
 transition-duration: 0.2s;
 text-shadow: 2px 2px 0 #42964c;
 outline: 0;
 }
button:hover{ 
 transform: translateY(5px);
 box-shadow: 0 0 0 #42964c;
}
button:active{
 transform: scale(0.8);
}
.slide-container{
 position: relative;
 overflow: hidden;
}
.slide-cover{
 position: absolute;
 height: 100%;
 width: 100%;
 top: 0;
 left: 0;
 transition-duration: 0.2s;
 transform: translateY(0);
}
.slid-up{
 transform: translateY(-120%);
}
#timer-slide{
 background: #d0782a;
}

<section>
 <div class='start-container slide-container'>
 <button id="start">START</button>
 <div id="timer-slide" class='timer slide-cover slid-up'>
 <p id="timer-label" class='label'>Timer</p>
 <p id="timer" class='main'></p>
 </div>
</div>
<div class='problem'>
 <p class='label'>PROBLEM</p>
 <p id="question" class='main'></p>
</div>
<div class='lights'>
 <div id='green-light' class='light'></div>
 <div id='red-light' class='light'></div>
</div>
<div class='score-container'>
 <p class='label'>SCORE</p>
 <p id="score" class='main'></p>
</div>
</section>
<p class='label'>(Use Numbers on Keyboard)</p>

---

¹_{https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/parseInt#Parameters}

Answer 2

Some extra points.

I was a little slow and Sam beat me to the answer, and he has covered many of the points I had. However I can not agree with parseInt, it is much better to use Number to convert a String if you know it is already an int.

Your naming is very bad. Prime example is const ob = new Object(); much better as game

The whole thing encapsulated would improve portability, see rewrite.

Use object literal declarations to define objects.

You had something like...

const myObj = new Object();
myObj.prop1 = "A";
myObj.func1 = function(){ ... }

...is less noisy if declared as

const myObj = {
 prop1 : "A",
 func1() { ... },
}

Use textContent rather than innerHTML to set just pain text.

ES6 offers many features that you could have used to simplify the code. One of which is getters and setters. In the rewrite below I have used setters to change the score, check the answer, and start the countdowns

There where many places you could have used ternary expressions rather than if/else statements.

// as if else statement
if(foo) { bar = 1 }
else { bar = 2 }
// is less noise as ternary
bar = foo ? 1 : 2;

Some functions such as setHTML(text) are not worth the extra code. It is simplier to write an expression. If you did something apart from assignment then the function is worth it. But for simple assignment not worth it.

setHTML(element,text) {
 element.textContent = text;
}
setHTML(theElement, "the txt");
// is better inlined.
theElement.textContent = "theText";

The way you handle the timers is too complex. The 100ms resolution is too high. The two types of countdown require too much code. In the rewrite the timer is set by a setter. game.timer = "start" and the time is counted down then the correct function called. The game play timer is set with game.timer = "end"

The rewrite

The rewrite has reduced the code size by about 20%.

;(() => {
 var totalScore, currentAnswer, timerCount, timerFunction, flashColorTimer, inPlay;
 const questionEl = document.getElementById('question');
 const scoreEl = document.getElementById('score');
 const redEl = document.getElementById('red-light');
 const greenEl = document.getElementById('green-light');
 const timerEl = document.getElementById('timer');
 const slideEl = document.getElementById('timer-slide');
 const timerLabelEl = document.getElementById('timer-label');
 const randInt = range => Math.random() * range | 0;
 const isNumber = val => !isNaN(val);
 function flashColor(colorName) {
 clearTimeout(flashColorTimer);
 redEl.style.background = colorName === "red" ? "white" : "black";
 greenEl.style.background = colorName === "green" ? "white" : "black";
 flashColorTimer = setTimeout(flashColor, 500);
 }
 const game = ({
 init() {
 inPlay = false;
 document.getElementById('start').addEventListener("click", this.start);
 document.addEventListener('keypress', event => game.answer = event.key);
 return this;
 },
 set score(value) { // abstract score now is how much is scored not total score
 if(value !== 0){
 totalScore += value;
 flashColor(value < 0 ? "red" : "green");
 } else { totalScore = 0 }
 scoreEl.textContent = totalScore;
 game.newQuestion();
 },
 set answer(value) {
 if (inPlay && currentAnswer !== null && isNumber(value)) {
 game.score = (currentAnswer === Number(value)) ? 100 : -50;
 }
 },
 set timer(type) {
 const start = type === "start";
 timerCount = start ? 4 : 11; // Add one as first tick is 0ms
 timerFunction = start ? game.begin : game.end;
 timerLabelEl.textContent = start ? "Starting in" : "Remaining";
 setTimeout(game.tick, 0);
 },
 tick() {
 timerCount -= 1;
 timerEl.textContent = timerCount;
 timerCount === 0 ?
 timerFunction() :
 setTimeout(game.tick, 1000);
 },
 start() {
 if (!inPlay) {
 slideEl.classList.remove('slid-up');
 currentAnswer = null;
 game.timer = "start";
 scoreEl.textContent = "0";
 inPlay = true;
 }
 },
 begin() {
 game.score = 0;
 game.timer = "end";
 },
 end() {
 inPlay = false;
 questionEl.textContent = "";
 slideEl.classList.add('slid-up');
 },
 newQuestion() {
 currentAnswer = randInt(10);
 const r1 = randInt(10);
 const r2 = randInt(10);
 questionEl.textContent = `${r1} + ${r2} - ${r1 + r2 - currentAnswer}`;
 },
 }).init();
})();

body{
 display: flex;
 flex-flow: column;
 justify-content: center;
 align-items: center;
 text-align: center;
 min-height: 70vh;
 background: #1d1f20;
 color:white;
}
div{
 display: flex;
 justify-content: center;
 align-items: center;
 flex-flow: column nowrap;
}
section{
 border: 2px solid white;
}
section > div{
 margin: 5px;
 height: 4em;
 width: 10em;
 border-radius: 5px;
}
.score-container{
 background: #568bbd;
}
div.problem{
 background: #86b38a;
}
p.main{
 font-size: 1.4rem;
 margin: 5px;
}
p.label{
 font-size: 0.8rem;
 margin: 2px;
}
.lights{
 flex-flow: row nowrap;
}
.light{
 height: 20px;
 width: 20px;
 border-radius: 50%;
 margin: 5px;
}
.light::after {
 content: '';
 width: 100%;
 height: 100%;
 border-radius: 50%;
 position: relative;
 z-index: 2;
}
#green-light::after{
 background: #00ff00aa;
}
#red-light::after{
 background: #ff0000aa;
}
.white{
 background: white;
}
button{ 
 border: 2px solid #42964c;
 background-color: #86b38a;
 color: white;
 border-radius: 10px;
 padding: 0.5em;
 font-size: 1.4rem;
 cursor: pointer;
 box-shadow: 0 5px 0 #42964c;
 transition-duration: 0.2s;
 text-shadow: 2px 2px 0 #42964c;
 outline: 0;
 }
button:hover{ 
 transform: translateY(5px);
 box-shadow: 0 0 0 #42964c;
}
button:active{
 transform: scale(0.8);
}
.slide-container{
 position: relative;
 overflow: hidden;
}
.slide-cover{
 position: absolute;
 height: 100%;
 width: 100%;
 top: 0;
 left: 0;
 transition-duration: 0.2s;
 transform: translateY(0);
}
.slid-up{
 transform: translateY(-120%);
}
#timer-slide{
 background: #d0782a;
}

<section>
 <div class='start-container slide-container'>
 <button id="start">START</button>
 <div id="timer-slide" class='timer slide-cover slid-up'>
 <p id="timer-label" class='label'>Timer</p>
 <p id="timer" class='main'></p>
 </div>
</div>
<div class='problem'>
 <p class='label'>PROBLEM</p>
 <p id="question" class='main'></p>
</div>
<div class='lights'>
 <div id='green-light' class='light'></div>
 <div id='red-light' class='light'></div>
</div>
<div class='score-container'>
 <p class='label'>SCORE</p>
 <p id="score" class='main'></p>
</div>
</section>
<p class='label'>(Use Numbers on Keyboard)</p>

• Sᴀᴍ Onᴇᴌᴀ: I also thought about mentioning ternary operators to simplify things, but forgot - there are so many things to remember! Good job eliminating the start button constant since it is only referenced once. Also good use of class setters!
• Roland Illig: I don't like the idea of "setting the timer to start". In my opinion starting and stopping the timer are fundamentally different, and they feel more like actions than like properties. Therefore I prefer having two functions startTimer and endTimer.
• Blindman67: @RolandIllig ????? start starts the countdown to the start of the game, and end starts the countdown to the end of game , keeping with the OPs names. I see the two actions as identical, start a countdown.