JavaScript/jQuery math game

Question

I wrote this entire program earlier today, and would like to get it reviewed.

It is an arcade style math game, where you are presented with two expressions on the left and right side. You will determine whether or not the first expression is less than, equal to or greater than the second expression.

If you get the question right, two new expressions will pop up. If you get it wrong, or you spend more than 5 seconds, you will lose. As you level up, the game gets harder and harder.

I will do my best to breakdown the code.

var scoreIndicator = document.getElementById("score");
var levelIndicator = document.getElementById("level");
var gameBlock = document.getElementsByClassName("game-block")[0];
var controls = document.getElementsByClassName("controls")[0];

var score = 0;
var level = 1;

var colors = ["red", "orange", "yellow", "green", "lightblue"];

I'm creating multiple arrays to store numbers here.

var easyNums = [];
var medNums = [];
var hardNums = [];
var extremeNums = [];

var operators = ["+", "-", "*", "/"];

I will fill these arrays with different numbers.

easyNums is designed for the lowest difficulty and only consists of positive digits between 1 and 10.

medNums includes all the digits that easyNums` does plus negative numbers greater than -10. It also includes all positive numbers less than 20

hardNums contains all numbers between -100 and 100.

extremeNums contains all triple digit numbers, positive and negative.

for (var i = 1; i <= 10; i++) {
    easyNums.push(i);
    medNums.push(i);
    medNums.push(-i);
    hardNums.push(i);
    hardNums.push(-i);
    extremeNums.push(i);
    extremeNums.push(-i);
}

for (var i = 11; i < 20; i++) {
    medNums.push(i);
    medNums.push(-i);
    hardNums.push(i);
    hardNums.push(-i);
    extremeNums.push(i);
    extremeNums.push(-i);
}

for (var i = 21; i < 100; i++) {
    hardNums.push(i);
    hardNums.push(-i);
    extremeNums.push(i);
    extremeNums.push(-i);
}

for (var i = 100; i < 1000; i++) {
    extremeNums.push(i);
    extremeNums.push(-i);
}

Here I'm creating functions to generate a random number from the number sets. At higher levels, the expressions will not contain just addition, but also subtraction, multiplication and division. To add to the difficulty, the expressions will also have random colors.

function randEasy() {
    return easyNums[Math.floor(Math.random() * easyNums.length)];
}

function randMed() {
    return medNums[Math.floor(Math.random() * medNums.length)];
}

function randHard() {
    return hardNums[Math.floor(Math.random() * hardNums.length)];
}

function randExtreme() {
    return extremeNums[Math.floor(Math.random() * extremeNums.length)];
}

function randOperation() {
    return operators[Math.floor(Math.random() * operators.length)];
}

function randColor() {
    return colors[Math.floor(Math.random() * colors.length)];
}

As the name suggests, this function generates both the left-hand and right-hand expressions. Each expression contains two numbers and an operand. Because I will be removing and re-adding expressions, I'm using classes and not ids.

I'm also applying the styles to both expressions.

function generateExpressions() {
    var exp1 = document.createElement("div");
    var exp2 = document.createElement("div");

    [exp1, exp2].forEach(function(exp,index) {
        exp.classList.add("exp" + (index + 1));
        exp.style.color = "white";
        exp.style.background = randColor();
        exp.style.textAlign = "center";
        exp.style.padding = "8px 0";
        gameBlock.appendChild(exp);

I mentioned earlier that the game gets harder and harder the longer you stay alive. At first, you will get easy questions you will be able to answer. As you level up, you're now getting harder questions from the harder numbersets.

        if (level == 1) {
            exp.innerHTML = "" + randEasy() + " + " + randEasy();
        }
        else if (level == 2 || level == 3) {
            exp.innerHTML = "" + randMed() + " + " + randMed();
        }
        else if (level == 4 || level == 5) {
            exp.innerHTML = "" + randHard() + " + " + randHard();
        }
        else if (level == 6 || level == 7) {
            exp.innerHTML = "" + randHard() + randOperation() + randHard();
        }
        else {
            exp.innerHTML = "" + randExtreme() + randOperation() +
            randExtreme();
        }
    });

}

This code is used to create the buttons. I think it's pretty self-explanatory

function createButtons() {
    var buttonGroup = document.createElement("div");
    buttonGroup.className = "button-group";
    controls.appendChild(buttonGroup);

    var greaterThan = document.createElement("button");
    greaterThan.classList.add("greaterThan");
    var equals = document.createElement("button");
    equals.classList.add("equals");
    var lessThan = document.createElement("button");
    lessThan.classList.add("lessThan");

    greaterThan.innerHTML = ">";
    equals.innerHTML = "=";
    lessThan.innerHTML ="<";

    buttonGroup.appendChild(greaterThan);
    buttonGroup.appendChild(equals);
    buttonGroup.appendChild(lessThan);
}

This is used to increase the score. If you got a problem right, you will gain 1 point. The existing expressions will be removed and replaced by new expressions which will be animated. The score will be checked to see if you met the requirements for leveling up.

function incrementScore() {
    score++;
    scoreIndicator.innerHTML = "<strong>Score: </strong>" + score;
    $(".exp1, .exp2").remove();
    generateExpressions();
    animateDivs();
    incrementLevel();
}

This is the function for leveling up. I thought a switch statement would work best here.

function incrementLevel() {
     switch(true) {
         case (score < 10):
             level = 1;
             break;
         case (score >= 10 && score < 20):
             level = 2;
             break;
         case (score >= 20 && score < 30):
             level = 3;
             break;
         case (score >= 30 && score < 40):
            level = 4;
            break;
         case (score >= 40 && score < 50):
            level = 5;
            break;
         case (score >= 50):
            level = 6;
            break;
     }
     levelIndicator.innerHTML = "<strong>Level: </strong>" + level;
}

These functions are used to evaluate the results. There is one for each of the three buttons.

$("body").on("click", ".lessThan", function() {
    var num1 = eval($(".exp1").text());
    var num2 = eval($(".exp2").text());
    (num1 < num2) ? incrementScore() : endGame();
});

$("body").on("click", ".equals", function() {
    var num1 = eval($(".exp1").text());
    var num2 = eval($(".exp2").text());
    (num1 == num2) ? incrementScore() : endGame();
});

$("body").on("click", ".greaterThan", function() {
    var num1 = eval($(".exp1").text());
    var num2 = eval($(".exp2").text());
    (num1 > num2) ? incrementScore() : endGame();
});

This function is used to start a new game. Again, I think it's fairly self-explanatory.

function startGame() {
    score = 0;
    level = 1;
    generateExpressions();
    animateDivs();
    scoreIndicator.innerHTML ="<strong>Score: </strong>" + score;
    levelIndicator.innerHTML = "<strong>Level: </strong>" + level;
    createButtons();
}

When a game ends, we will remove all the game elements.

function endGame() {
    $(".exp1, .exp2, .lessThan, .equals, .greaterThan").remove();
}

This is where the animation and timing function comes in.

function animateDivs() {
    $(".exp2").animate( { "marginRight": "70%" }, 5000);
    var previousScore = score;
    setTimeout(function() {
    if (score == previousScore) {
        endGame()
    }
  }, 5000);
}

I really worked hard creating this post, because I want to analyze my code and improve as a developer. I put a lot of effort into this post.

You guys are welcome to try out the game here.

https://jsfiddle.net/mypkktu7/

Answer 1

This really isn't a full answer, but apparently comments are frowned upon here.

• Instead of writing 4 for loops, consider writing one with if-then statements.

• Instead of using four arrays (easyNums, medNums, etc), consider using an array of arrays.

• Instead of having four rand functions, (randEasy, randMed, etc), consider making the difficulty level a parameter to a single function, or even just a general random-member-of-array function, where the array itself is passed.

Answer 2

Readability and styling

For the most part, the indentation, use of double quotes and whitespace around operators seem consistent. There is one place (see below) where you continue an expression on the next line. Someone glancing over the code can assume that randExtreme() is it's own call instead of part of the previous expression. I would recommend adding extra indentation when continueing an expression on the next line.

exp.innerHTML = "" + randExtreme() + randOperation() +
randExtreme();

Eval

Never ever use eval. Eval allows for arbitrary code execution. Even though you create the elements yourself, you cannot protect yourself from elements that are created by something else elsewhere on the page. Always use an alternative. In your case you can precompute the result and store it in a data-attribute.

$( selector ).data( "result", 1337 );

Semantics

You are using classes for elements you expect to only have at most 1 of. Use id's instead for elements that should only ever appear once.

In incrementLevel(..) you abuse the switch-statement. The whole point of the switch statement is to use an appropriate case for the variable in the head of that construction. You should use an if-elseif-else construction instead. In that case you can remove several conditions too, considering that score < 10 is guaranteed to be false if score < 20 is evaluated. Why are you not using Math.floor(..) and division to calculate the level number?

Styling is the task of CSS. U can use HTMLElement.style to change the style of some things via javascript, but generally you should not do this. Consider using an Array of possible classes you can use for the blocks, and randomly select one to add. Then put the styling in your CSS. Put common CSS in it's own class and apply that regardless.

Code duplication

See @BarryCarter's answer for more information on that.

Jquery vs plain javascript

I notice that you are using jQuery in your project, but only in very specific situations. Why use $(".exp1").text(), but use scoreIndicator.innerHTML instead of $(scoreIndicator).html( .. )? Consider using more jQuery if you want to use jQuery, or transform the remaining bits to plain javascript.

Wasting memory

You are loading hundreds of numbers into arrays for a series of functions that generate an integer between a continuous interval between a lower bound and an upper bound. You can replace this with a single function that takes a lower bound and an upper bound. To make it inclusive, you just need to increment max with 1.

function randBetween( min, max ) {
  //min included, max excluded
  return Math.floor( Math.random() * (max - min) ) + min;
}

Usability

From a usability aspect, white text on a yellow background is impossible to read. Similarly, it is harder to read moving text than it is to read static text. To signify how much time is left, consider using something like a progress bar instead.