# Maths Q&A game in JavaScript

This used to be university work now I put it our here for review purposes.

1. Could this be constructed better?
2. Is there any easier path, performance-wise?
3. Could this be optimised?
4. Which features and functionalities could/should be added to this game?

<!doctype html>
<html>

<title>Math.floor</title>

<body>

<h1> Math.floor: The Maths Game</h1>

<script>
// Universty work
// @author Darik
// Math.floor

var score = 0;
var times = 0;

function questions(e) {
var x = Math.floor(Math.random() * 11);
var y = Math.floor(Math.random() * 11);
var z = 0;
var sign = '';

if (e == 1) {
sign = '+'
z = x + y;
interact(x, sign, y, z)
} else if (e == 2) {
sign = '-'
z = x - y;
interact(x, sign, y, z)
} else if (e == 3) {
sign = '/'
z = x / y;
interact(x, sign, y, z)
} else {
sign = '*'
z = x * y;
interact(x, sign, y, z)
}

}

function interact(x, sign, y, z) {
times++;
var userAnswer = prompt("What is " + x + " " + sign + ' ' + y + "?");
if (z == userAnswer) {
document.write("<p>" + x + " " + sign + ' ' + y + " = " + userAnswer + " is correct </p>");
score = score + 1;
} else {
document.write("<p>" + x + " " + sign + ' ' + y + " = " + userAnswer + " <span> is INCORRECT. The correct answer is </span> " + z + "</p>");
}
}

function _score() {
if (times == score) {
document.write('<p> A perfect score </p>');
document.body.style.backgroundColor = "#FF99CC";
} else {
document.body.style.backgroundColor = "#CCFFFF";
}
document.write('<p> You have answerd ' + score + ' questions out of ' + times + '</p>');

}

questions(2);
questions(1);
questions(4);
questions(3);
_score()
</script>
</body>

</html>

You ask some unreasonable division questions. That is answerable by the special string Infinity, but not infinity. The expected answer is NaN, but responding with NaN will not work. 10 / 3 = 3.33333333 is INCORRECT. The correct answer is 3.3333333333333335

• Don't put your Java Script code into the HTML document, always use separate files.

• Don't create global variable. Either put all your code into a "namespace" object, or at least wrap it in a immediately involked function.

• The parameter e in questions(e) is a terrible choice for multiple reasons:

• The name itself is far too short and gives absolute no information what it's for.
• You utilize "magic numbers" that have absolute no connection to its use. What does the number 1 have to do with addition?
• NEVER EVER, EVER use document.write (as long as you don't know what it does). You are very, very lucky that your code actually works, because with document.write used like you are doing it here, you are overwriting the whole document including the JavaScript code itself and code that doesn't exist can't be executed.

See below for a re-worked version of the code for your consideration. I would suggest that this version has been structured in a clearer manner.

There is also the following added functionality:

• Start button so that the quiz doesn't begin immediately on page load
• Recognition in written feedback of questions that are essentially skipped by pressing the 'escape' key or submitting a blank answer during the prompt

Preview

<!doctype html>
<html>

<title>Math.floor</title>

<style>
#quiz-feedback {
float: left;
width: 400px;
min-height: 180px;
border: 2px solid #efefef;
}
</style>

<script>

var scorecard = {
correct: 0,
incorrect: 0,
feedback: []
};
var quiz = [];

function initialize() {
var chosenTopics = ['-', '+', '*', '/'];

// Generate quiz questions
for(var i = 0; i < chosenTopics.length; i++) {
quiz.push(createQuestion(chosenTopics[i]));
}

startGame(quiz);
}

function createQuestion(operator) {
var x = Math.floor(Math.random() * 11);
var y = Math.floor(Math.random() * 11);

switch(operator) {
case '+': {
answer = x + y;
break;
}
case '-': {
answer = x - y;
break;
}
case '*': {
answer = x * y;
break;
}
case '/': {
answer = x / y;
break;
}
}

var question = x + ' ' + operator + ' ' + y;

return {
questionText: 'What is ' + question + '?',
question: question,
}
}

function startGame(quiz) {
resetGame();

for(var i = 0; i < quiz.length; i++) {

var userAnswer = prompt(quiz[i].questionText);
var feedback = quiz[i].question;

scorecard.correct++;
feedback = feedback + ' = ' + userAnswer + ' is correct';
} else if(userAnswer === null || userAnswer === '') {
scorecard.incorrect++;
feedback = feedback + ' was skipped. The correct answer is ' + quiz[i].answer;
} else {
scorecard.incorrect++;
feedback = feedback + ' = ' + userAnswer + ' is INCORRECT. The correct answer is ' + quiz[i].answer;
}

scorecard.feedback.push(feedback);
updateFeedbackDisplay();

}

var verdict = 'You have answered ' + scorecard.correct + ' questions out of ' + quiz.length + ' correctly.';
scorecard.feedback.push(verdict);
updateFeedbackDisplay();
}

function updateFeedbackDisplay() {
var feedbackBox = document.getElementById('quiz-feedback');
var outputHTML = '';
for(var i = 0; i < scorecard.feedback.length; i++) {
outputHTML = outputHTML + '<p>' + scorecard.feedback[i] + '</p>';
}
feedbackBox.innerHTML = outputHTML;
}

function resetGame() {
quiz = [];

scorecard.correct = 0;
scorecard.incorrect = 0;
scorecard.feedback = [];

var feedbackBox = document.getElementById('quiz-feedback');
feedbackBox.innerHTML = '';
}

</script>

<body>
<h1> Math.floor: The Maths Game</h1>

<p><a href="#" onclick="initialize();">Start Quiz</a></p>
<div id="quiz-feedback"</div>
</body>

</html>


Reasons why:

Primarily the code has been separated out into more clearly-named functions, each responsible for a separate contribution to the workings of the app, e.g. createQuestion(), resetGame(), etc.

This is instead of code that is related to for example both updating the score and rendering the next question on the screen in the same function, as is the case with the original code in the function interact() (now found separately in startGame() and updateFeedbackDisplay()).

If you want to either change the scoring system, change the format of the questions, or change how the game is presented, it's much easier to manage if the code for these separated aspects are not mingled together. They are now separate. It also means that changes are less error prone, with this approach you are much more likely to be able to change one function without affecting the rest of the app, unlike before. In other words, the code is a bit more modular/loosely-coupled between functions and therefore easier to scale (two very important principles of good development).

Other reasons why:

• In specifying the type of questions the developer wants, if there's a 1-to-1 mapping between a number and the maths operator as seen in the original code, then the number is irrelevant and it would be clearer to use the operator directly:

• Before: questions(1); questions(4); ...
• After: var chosenTopics = ['-', '+', '*', '/']; // Add as many operator string values to this array as desired
• The document.write method, commented upon by other users, has now been withdrawn. Instead the feedback messages are gathered inside an array (scorecard.feedback) and after each question the contents of this array is cycled through, pieced together, and set as the HTML of the div element "quiz-feedback" (see updateFeedbackDisplay() function)

• The above version implements the switch statement suggestion from Snowbody

• The above version uses more meaningful variable names as pointed out by RoToRa

Apologies for not including my reasoning originally, hope this helps.

        if (e == 1) {
sign = '+'
z = x + y;
interact(x, sign, y, z)
} else if (e == 2) {
sign = '-'
z = x - y;
interact(x, sign, y, z)
} else if (e == 3) {
sign = '/'
z = x / y;
interact(x, sign, y, z)
} else {
sign = '*'
z = x * y;
interact(x, sign, y, z)
}


You could use a "switch" here to eliminate the multiple ifs, but I think a better item to pass it would be an object consisting of the addition function and the proper sign string.