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

My JavaScript calculator

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
javascriptcalculatorstackoverflow

Problem

I am playing a bit with vanilla JavaScript and created a simple JavaScript calculator using eval(). Any suggestions for improvements?

var keys = document.getElementsByClassName('number'),
    getTotal = document.getElementById('showTotal'),
    btnReset = document.getElementById('reset'),
    visor = document.getElementById('visor');

for (var i = 0; i 
    
     

       
       
        

       
      

  

Solution

A few notes:

-
Shy away from eval(). TL;DR: impacts performance and is a security risk. I don't know a better method to use for vanilla JavaScript however, so we'll have to stick with it for now.

-
Put your operators in an array:

var operators = ['+', '-', 'x', '÷'];


-
Right now your code doesn't handle decimal points very well. I would add a Boolean flag to track if you have them in the number.

-
You don't consider the possibility of a user entering x or ÷ into your text field. You should replace those with their respecting proper operators:

equation = equation.replace(/x/g, '*').replace(/÷/g, '/');


-
What happens if the last character of the equation is an operator or a decimal? We should remove it and then process the equation:

if(operators.indexOf(lastChar) > -1 || lastChar == '.')
        equation = equation.replace(/.$/, '');


-
Let's consider failure cases where we don't want to process the equation. I've implemented some checks handling these in the final code with comments explaining them.

-
If we start with an operator other than -

-
If there is more than one decimal in the number

-
Two operators appear together

Final Results



// Get all the keys from document
var keys = document.querySelectorAll('#calculator span');
var operators = ['+', '-', 'x', '÷'];
var decimalAdded = false;

// Add onclick event to all the keys and perform operations
for (var i = 0; i -1 || lastChar == '.')
equation = equation.replace(/.$/, '');

if (equation)
input.innerHTML = eval(equation);

decimalAdded = false;
}

// Basic functionality of the calculator is complete. But there are some problems like
// 1. No two operators should be added consecutively.
// 2. The equation shouldn't start from an operator except minus
// 3. not more than 1 decimal should be there in a number

// We'll fix these issues using some simple checks

// indexOf works only in IE9+
else if (operators.indexOf(btnVal) > -1) {
// Operator is clicked
// Get the last character from the equation
var lastChar = inputVal[inputVal.length - 1];

// Only add operator if input is not empty and there is no operator at the last
if (inputVal != '' && operators.indexOf(lastChar) === -1)
input.innerHTML += btnVal;

// Allow minus if the string is empty
else if (inputVal === '' && btnVal === '-')
input.innerHTML += btnVal;

// Replace the last operator (if exists) with the newly pressed operator
if (operators.indexOf(lastChar) > -1 && inputVal.length > 1) {
// Here, '.' matches any character while $ denotes the end of string, so anything (will be an operator in this case) at the end of string will get replaced by new operator
input.innerHTML = inputVal.replace(/.$/, btnVal);
}

decimalAdded = false;
}

// Now only the decimal problem is left. We can solve it easily using a flag 'decimalAdded' which we'll set once the decimal is added and prevent more decimals to be added once it's set. It will be reset when an operator, eval or clear key is pressed.
else if (btnVal === '.') {
if (!decimalAdded) {
input.innerHTML += btnVal;
decimalAdded = true;
}
}

// if any other key is pressed, just append it
else {
input.innerHTML += btnVal;
}

// prevent page jumps
e.preventDefault();
}
}

`/ Basic reset /

* {
margin: 0;
padding: 0;
box-sizing: border-box;
/ Better text styling /
font: bold 14px Arial, sans-serif;
}
/ Finally adding some IE9 fallbacks for gradients to finish things up /

/ A nice BG gradient /

html {
height: 100%;
background: white;
background: radial-gradient(circle, #fff 20%, #ccc);
background-size: cover;
}
/ Using box shadows to create 3D effects /

#calculator {
width: 325px;
height: auto;
margin: 100px auto;
padding: 20px 20px 9px;
background: #9dd2ea;
background: linear-gradient(#9dd2ea, #8bceec);
border-radius: 3px;
box-shadow: 0px 4px #009de4, 0px 10px 15px rgba(0, 0, 0, 0.2);
}
/ Top portion /

.top span.clear {
float: left;
}
/ Inset shadow on the screen to create indent /

.top .screen {
height: 40px;
width: 212px;
float: right;
padding: 0 10px;
background: rgba(0, 0, 0, 0.2);
border-radius: 3px;
box-shadow: inset 0px 4px rgba(0, 0, 0, 0.2);
/ Typography /
font-size: 17px;
line-height: 40px;
color: white;
text-shadow: 1px 1px 2px rgba(0, 0, 0, 0.2);
text-align: right;
letter-spacing: 1px;
}
/ Clear floats /

.keys,
.top {
overflow: hidden;
}
/ Applying same to the keys /

.keys span,
.top span.clear {
float: left;
position: relative;
top: 0;
cursor: pointer;
width: 66px;
height: 36px;
background: white;
border-radius: 3px;
box-shadow: 0px 4px rgba(0, 0, 0, 0.2);
margin: 0 7px 11px 0;
color: #888;
line-height: 36px;
text-align: center;
/* prevent

Code Snippets

var operators = ['+', '-', 'x', '÷'];
equation = equation.replace(/x/g, '*').replace(/÷/g, '/');
if(operators.indexOf(lastChar) > -1 || lastChar == '.')
        equation = equation.replace(/.$/, '');

Context

StackExchange Code Review Q#129096, answer score: 3

Revisions (0)

No revisions yet.