patternjavascriptMinor
Beginner's JavaScript calculator
Viewed 0 times
javascriptbeginnercalculator
Problem
As a part of a study-group I did a very simple little JavaScript calculator. It's not made to look pretty.
Any ideas on how I can make the code prettier?
`/*
* This is a simple JS calculator
* Made by Gemtastic 2016-05-08
*/
// Gather and set up DOM elements
var numberScreen = 'screen';
var numBtns = document.getElementsByClassName('number');
var operators = document.getElementsByClassName('operator');
// Mini statemachine of if we're in the middle of operating on a number.
var operating = false;
var operand = '';
/*
* Clears the content from the screen.
*/
function clearScreen() {
document.getElementById(numberScreen).innerHTML = "0";
resetOperating();
}
/*
* Evaluates the entered numbers with the given operator.
*/
function evaluate() {
var result;
/* If it isn't operating or the last entered is the operator
* there's no point in doing anything.
*/
if(operating && isLastEnteredNumber()){
var content = document.getElementById(numberScreen).innerHTML;
var numbers = content.split(operand);
// Parse strings to be able to operate on them.
var firstNumber = parseInt(numbers[0], 10);
var secondNumber = parseInt(numbers[1], 10);
// Switch the stored operator.
switch(operand) {
case '+':
result = firstNumber + secondNumber;
break;
case '÷':
if(firstNumber !== 0 && secondNumber !== 0) {
result = firstNumber / secondNumber;
} else {
result = 0;
}
break;
case '×':
result = firstNumber * secondNumber;
break;
case '–':
result = firstNumber - secondNumber;
break;
default:
console.log('Something went terribly wrong! ' + operand + " is not a supported operator!");
}
}
resetOperating();
document.getElementById(numberScreen).innerHTML = result || 0;
return result || 0;
}
/*
* Transforms the current input or the input's value if it's a
* full expression but not evalua
Any ideas on how I can make the code prettier?
`/*
* This is a simple JS calculator
* Made by Gemtastic 2016-05-08
*/
// Gather and set up DOM elements
var numberScreen = 'screen';
var numBtns = document.getElementsByClassName('number');
var operators = document.getElementsByClassName('operator');
// Mini statemachine of if we're in the middle of operating on a number.
var operating = false;
var operand = '';
/*
* Clears the content from the screen.
*/
function clearScreen() {
document.getElementById(numberScreen).innerHTML = "0";
resetOperating();
}
/*
* Evaluates the entered numbers with the given operator.
*/
function evaluate() {
var result;
/* If it isn't operating or the last entered is the operator
* there's no point in doing anything.
*/
if(operating && isLastEnteredNumber()){
var content = document.getElementById(numberScreen).innerHTML;
var numbers = content.split(operand);
// Parse strings to be able to operate on them.
var firstNumber = parseInt(numbers[0], 10);
var secondNumber = parseInt(numbers[1], 10);
// Switch the stored operator.
switch(operand) {
case '+':
result = firstNumber + secondNumber;
break;
case '÷':
if(firstNumber !== 0 && secondNumber !== 0) {
result = firstNumber / secondNumber;
} else {
result = 0;
}
break;
case '×':
result = firstNumber * secondNumber;
break;
case '–':
result = firstNumber - secondNumber;
break;
default:
console.log('Something went terribly wrong! ' + operand + " is not a supported operator!");
}
}
resetOperating();
document.getElementById(numberScreen).innerHTML = result || 0;
return result || 0;
}
/*
* Transforms the current input or the input's value if it's a
* full expression but not evalua
Solution
Interesting question,
- I noticed that 9-3*3 does not return 0, but 27, you are not following PEMDAS
- Personally, I would make sure that the input only contains numbers, dots, and math operators, and then use
eval.eval("9-3*3")does return correctly 0.
- 0.0022 * 2 returns zero.
- I would assign html elements like the one found from
document.getElementById(numberScreen)to a higher scope variable for readability and performance
- You do not check for existing periods in the current number, allowing for numbers like 12.24.2016
- Using a pure UI function (
innerHTML) to drive the logic is wrong. It's okay for a beginner class, but anything more advanced should drive of (id). You dont want to replace 'x' with '*' and also having to change the JS code
- Your indentation is not perfect, use http://jsbeautifier.org/
- You have zero warnings on jshint.com, which is amazing
Context
StackExchange Code Review Q#150510, answer score: 6
Revisions (0)
No revisions yet.