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

Beginner's JavaScript calculator

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

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.