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

Basic web calculator

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

Problem

I've built a calculator. I would not use angular or bootstrap because I am becoming addicted to them. I also wanted to check for double '.' use, and to limit the screens max text length but did not make it in time.

I am new to JavaScript, CSS and HTML and would like to know how I am doing.

```



welcome!


7
8
9
+
-
C



4
5
6
*
/



1
2
3

%
(
)



0

,
=

//regular expression for input controll and users value holder.
var rg = new RegExp(/\d/);
var codestr = "";
//"event listener" for button presses and input controll
var code = function(str){
var tested = rg.test(codestr.substring(codestr.length-1));
var sstring = (codestr.substring(codestr.length-1));
(str=="." && !tested ?
codestr = codestr+"0"+str : ((rg.test(str) || str=="(" || str==")") ?
codestr = codestr+str : (tested || sstring=="(" || sstring==")") ?
codestr= codestr+str: restring(str) )) ;
document.getElementById('screen').innerHTML = codestr;
};
function restring(str){ codestr = (codestr.substring(0,codestr.length -1))+str };
//user input interpreter ( calculator logic )
var compiler = function(){
var rezPlz = new Function('return '+codestr)();
codestr = ""+rezPlz;
document.getElementById('screen').innerHTML = codestr;
};
//clear button
function clr(){codestr="";
document.getElementById('screen').innerHTML = "0";}

button{
font-size:40px;
background-color: #FAFAFA;
}
.cont{
margin: 5px;
padding-top:6px
}
.bt{
display: inline;
padding: 0 5px 0 5px;
margin-top: 0px;
border: solid;
border-width: 1px;
border-color: blue;
}
.bt:hover{
background-color: darkblue;
color:#F0F0F0;
}
.bordered{
border:solid;
border-width: 2px;
border-color : blue;
outline:solid;

Solution

Event Delegation

You have soooo many events being attached to your element when you really only need one. Yes, one. It's a technique called event delegation.

document.getElementById('buttons').addEventListener('click', doStuff);

function doStuff(e) {
    if (e.target.tagName.toLowerCase() == 'button') {
        if (e.target.innerHTML == 'C') {
            clr();
        } else {
            code(e.target.innerHTML);
        }
    }
}

function code(operator) {
    console.log(operator);
}


Markup:


    
        7
        8
        9
        +
        -
        C
    
    


Unsemantic class names

The naming conventions you use are baffling. I would guess that bt is supposed to be an abbreviation for button, but it doesn't make any sense at all when you see that you're also using an element selector to style all buttons (and the only buttons on the page all have the bt class attached to them). I have no idea what cont is supposed to be.

The bordered class just describes the way the element looks, not what it's purpose is. What happens when your calculator needs a facelift and the element shouldn't be bordered anymore?

Code Snippets

document.getElementById('buttons').addEventListener('click', doStuff);

function doStuff(e) {
    if (e.target.tagName.toLowerCase() == 'button') {
        if (e.target.innerHTML == 'C') {
            clr();
        } else {
            code(e.target.innerHTML);
        }
    }
}

function code(operator) {
    console.log(operator);
}
<fieldset id="buttons">
    <div>
        <button>7</button>
        <button>8</button>
        <button>9</button>
        <button>+</button>
        <button>-</button>
        <button>C</button>
    </div>
    <!-- etc... -->
</fieldset>

Context

StackExchange Code Review Q#100643, answer score: 5

Revisions (0)

No revisions yet.