HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Simple math facts game

Submitted by: @import:stackexchange-codereview··
0
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 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 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 }


| ̄ ̄ ̄ ̄ ̄ ̄ |
| 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.