patternjavascriptMinor
Simple math facts game
Viewed 0 times
gamesimplemathfacts
Problem
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
I'd like feedback on how to improve the code, particularly if there's a better way of doing the
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';
}Solution
From a once over:
-
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:
-
Here's my attempt for fun and giggles.
- You are using lowerCamelCase, that is good
- Variables like
ansandprobdeserve to be written out toanswerandproblem
-
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 placesvar 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 neednum1 = randInt(maxNum) + 1;
- Avoid dropping the curly braces and newlines, even when
ifstatements 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 }
| ̄ ̄ ̄ ̄ ̄ ̄ |
| What is |
| |
| ? |
| _______|
(\__/) ||
(•ㅅ•) ||
/ づ
Code Snippets
while( stillPlaying ){
problem = generateNewProblem();
answer = getAnswer( problem );
processAnswer( answer );
}var plusSign = '+',
minusSign = '-',
timesSign = '*,
divideSign = '/',
signs = [plusSign , minusSign, timesSign, divideSign];
if (sign == divideSign) num1 *= num2;Context
StackExchange Code Review Q#69178, answer score: 5
Revisions (0)
No revisions yet.