Simple math facts game

Question

I've made a simple game for testing math facts (think practice for elementary school-age kids). It displays a random addition, subtraction, multiplication, or division problem, and feedback on if the right answer was entered.

For now it works interacts with the user via prompt(), but I plan to add a GUI later. You can exit out of the infinite prompt() loop by clicking cancel.

I'd like feedback on how to improve the code, particularly if there's a better way of doing the getCorrectAns() function, and if I'm using best practices.

var maxNum = 12, stillPlaying = true, ans, isCorrect = '', correctAns = '', sign, num1 = 0, num2 = 0, prob, correctAns; function randInt(max) { return Math.floor(Math.random() * max); } function getCorrectAns(n1, sign, n2) { switch (sign) { case '+': return n1 + n2; case '\u2212': return n1 - n2; case '\xD7': return n1 * n2; case '\xF7': return n1 / n2; default: return false; } } while (stillPlaying) { sign = ['+', '\u2212', '\xD7', '\xF7'][randInt(4)]; num1 = randInt(maxNum + 1); num2 = randInt(maxNum + 1); if (sign == '\u2212') num1 += num2; if (sign == '\xF7') { num2 = randInt(maxNum) + 1; num1 *= num2; } prob = num1 + ' ' + sign + ' ' + num2; ans = prompt(isCorrect + prob); if (!ans) stillPlaying = false; correctAns = getCorrectAns(num1, sign, num2); isCorrect = (correctAns == ans) ? 'Correct! ' : 'Incorrect, ' + prob + ' = ' + correctAns + '.\n'; }

Incorrect, 0 ÷ 0 = NaN. I think division by zero should be excluded! — Marc-Andre
– Marc-Andre, Commented Nov 7, 2014 at 17:38

konijn · Accepted Answer · 2014-11-08 13:04:59Z

From a once over:

You are using lowerCamelCase, that is good
Variables like ans and prob deserve to be written out to answer and problem
You are using functions, but I think it can be split up a bit more from a best practices. I would go for something like this:
```
while( stillPlaying ){ problem = generateNewProblem(); answer = getAnswer( problem ); processAnswer( answer ); } 
```
['+', '\u2212', '\xD7', '\xF7'][randInt(4)]; I wonder why you could not simply use ['+','-','*','/']. Regardless you should have 4 constants for those values since you use those values repeatedly in other places
```
var plusSign = '+', minusSign = '-', timesSign = '*, divideSign = '/', signs = [plusSign , minusSign, timesSign, divideSign]; if (sign == divideSign) num1 *= num2; 
```
This seems funny: num1 = randInt(maxNum + 1);, if you were trying to avoid zero, you really need num1 = randInt(maxNum) + 1;
Avoid dropping the curly braces and newlines, even when if statements are short like here: if (sign == '\u2212') num1 += num2;

Here's my attempt for fun and giggles.

var maxNum = 12, signWidt = 12, question = document.getElementById( 'question' ), input = document.getElementById( 'input' ), errorMessage = document.getElementById( 'errorMessage' ), greatMessage = document.getElementById( 'greatMessage' ), answer; //Some of these signs are closer to what children use in school var plusSign = '+', minusSign = '\u2212', timesSign = '\u2212', divideSign = '\xF7', signs = [plusSign , minusSign, timesSign, divideSign]; function randInt(max) { return Math.floor(Math.random() * max); } function calculateAnswer(n1, sign, n2) { switch (sign) { case plusSign: return n1 + n2; case minusSign: return n1 - n2; case timesSign: return n1 * n2; case divideSign: return n1 / n2; default: return false; } } function displayQuestion( s ){ //We know the question will never surpass 12 chars.. var extraneousSpace = 12 - s.length, spaceLeft, spaceRight; //Compare last bit to 1, if that bit is 1, the number is odd //So we subtract 1 before we evenly divide by 2 and 1 space on the right if( extraneousSpace & 1 ){ extraneousSpace--; spaceLeft = extraneousSpace / 2; spaceRight = spaceLeft + 1; } else { spaceLeft = spaceRight = extraneousSpace / 2; } //Mildly evil re-use of variable and magical space repeating routine //http://stackoverflow.com/questions/202605/repeat-string-javascript spaceLeft = new Array( spaceLeft + 1 ).join( ' ' ); spaceRight = new Array( spaceRight + 1 ).join( ' ' ); question.innerText = spaceLeft + s + spaceRight; } function generateQuestion(){ var sign = signs[randInt(4)], num1 = randInt(maxNum + 1), num2 = randInt(maxNum + 1); if (sign == minusSign){ num1 += num2; } if (sign == divideSign) { num2 = randInt(maxNum) + 1; num1 *= num2; } answer = calculateAnswer( num1 , sign, num2 ); displayQuestion( num1 + ' ' + sign + ' ' + num2 ) } generateQuestion(); input.focus(); input.addEventListener( 'keypress' , function(e){ console.log( this ); if( e.which == 13 || e.keyCode == 13 ){ if( input.value != answer ){ errorMessage.innerText = 'Wrong, ' + question.innerText.trim() + ' = ' + answer; greatMessage.innerText = ''; }else { errorMessage.innerText = ''; greatMessage.innerText = 'That is right, here\'s another question'; } input.value = ''; generateQuestion(); } });

#errorMessage { color: red } #greatMessage { color: green }

<pre> |￣￣￣￣￣￣ | | What is | |<span id='question'> </span>| | ? | | ＿＿＿＿＿__| (\__/) || (•ㅅ•) || / 　 づ </pre> <input type='text' id='input'><br> <pre> <span id='errorMessage'></span><span id='greatMessage'></span> </pre>

Thanks for the suggestions! A few clarifications: I used the actual signs because children will probably be unfamiliar with * and / as multiplication and division signs. Also, I am only trying to avoid zero as a denominator of a division problem; I have added a quick fix for that. — NinjaBearMonkey
– NinjaBearMonkey, Commented Nov 7, 2014 at 19:40

Stack Exchange Network

Simple math facts game

1 Answer 1

You must log in to answer this question.

Hot Network Questions

Simple math facts game

1 Answer 1

You must log in to answer this question.

Related

Hot Network Questions