Basic web calculator

Question

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.

 <html>
    <body style="font-size: 40px;  ">


<div style=" max-width: 350px;" class="bordered">
<div class="cont bordered"> <label style="max-width:340px;"  id="screen">welcome!</label> </div>

    <div class="cont">
        <button class="bt" onclick="code(7)">7</button>
        <button class="bt" onclick="code(8)">8</button>
        <button class="bt" onclick="code(9)">9</button>
        <button class="bt" style="margin-left: 50px; " onclick="code('+')">+</button>
        <button class="bt" onclick="code('-')">-</button>
<button class="bt" onclick="clr()">C</button>

        </div>
<div class="cont">
        <button class="bt" onclick="code(4)">4</button>
        <button class="bt" onclick="code(5)">5</button>
        <button class="bt" onclick="code(6)">6</button>
        <button class="bt" style="margin-left: 50px; " onclick="code('*')">*</button>
        <button class="bt" onclick="code('/')">/</button>

        </div>
<div class="cont">
        <button class="bt" onclick="code(1)">1</button>
        <button class="bt" onclick="code(2)">2</button>
        <button class="bt" onclick="code(3)">3</button>

        <button class="bt" style="margin-left: 50px; " onclick="code('%')">%</button>
        <button class="bt" onclick="code('(')">(</button>
        <button class="bt" onclick="code(')')">)</button>

    </div>
<div class="cont">
    <button class="bt" onclick="code(0)" style=" padding-right: 25px; padding-left: 25px;"> 0 </button>

        <button class="bt" style="; margin-left:2px; padding:0 10px 0 10px;" onclick="code('.')">,</button>
    <button class="bt" onclick="compiler()" style=" padding-right: 30px; padding-left: 30px; margin-left: 50px;"> = </button>
</div>

</div>
</body>

<script >
    //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";}

</script>

</html>
<style rel="stylesheet" type="text/css">
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; 
        outline-width:1px; 
        outline-color:aqua;
    }
</style>

I know it's not mobile friendly, but that's what I get for doing a no-bootstrap project in short time.

sorry, figured this was less spammy, I will go read full terms and the guidelines tonight. — Bellator, Aug 12, 2015 at 14:27

cimmanon · Accepted Answer · 2015-08-12 02:17:11Z

5

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:

<fieldset id="buttons">
    <div>
        <button>7</button>
        <button>8</button>
        <button>9</button>
        <button>+</button>
        <button>-</button>
        <button>C</button>
    </div>
    <!-- etc... -->
</fieldset>

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?

answered Aug 12, 2015 at 2:17

cimmanon

3,63414 silver badges32 bronze badges

\$\begingroup\$ Great job explaining event delegation - I'll update my answer to point to this instead. \$\endgroup\$
– Scott
Aug 12, 2015 at 2:20
\$\begingroup\$ it would make sense to have 2 event listeners for clear and others, though. \$\endgroup\$
– njzk2
Aug 12, 2015 at 3:18
\$\begingroup\$ @njzk2 Why? It makes sense to have separate functions for different behaviors, but I don't see why it makes sense to have separate event handlers. Less event handlers == more efficient. \$\endgroup\$
– cimmanon
Aug 12, 2015 at 10:55
\$\begingroup\$ Less event handlers == more efficient.? I don't see why? I would go document.getElementById('buttons').addEventListener('click', code); and document.getElementById('clear').addEventListener('click', clear); That's it. Just to avoid the ugly if (e.target.tagName.toLowerCase() === 'button') { \$\endgroup\$
– njzk2
Aug 12, 2015 at 13:41
\$\begingroup\$ also, what if you really had 10, 20, 100 different behaviors? that would be a long handler, not very readable \$\endgroup\$
– njzk2
Aug 12, 2015 at 13:42

| Show 1 more comment

Community · Accepted Answer · 2020-06-10 13:24:26Z

I applaud you for wanting to move away from frameworks you have come to rely on - it will definitely help you in the long run when it comes to debugging in the future!

Here are a few things I noticed, and that could be improved with your calculator.

HTML

Try to stay away from inline styles. Adding style attributes for single elements might seem like an easy way to quickly apply a style, but it makes your HTML messy and CSS hard to debug. Inline styles will override CSS, which eventually leads to using !important. You also don't give the browser the chance to cache that style, as it will for CSS documents which rarely change.
Try to avoid using onclick attributes to hook up Javascript events to elements. This also falls under the category of Don't Repeat Yourself (DRY), since a lot of the events call the code function. It's possible to handle this with one event and one event listener that looks at the innerText of the caller!
Courtesy of Ismael in the comments: You're missing a doctype! Fortunately, the HTML5 doctype is super easy. Just add <!doctype html> before your <html> tag.
You need to add a <title> tag to your head section, which gives the page the name that appears in the taskbar and tab of a user's web browser.
See if you can save your CSS in an external stylesheet (file.css for example) and reference it in the head section like so: <link rel="stylesheet" href="file.css">

Javascript

There's a loooot of ternary action happening in the code function to the point of making it unreadable. I'm not quite sure what this function even does at first glance - I have to format it extensively to begin to see how it works. It looks like it eventually creates a function from a string, which I think is a pretty clever way to solve the classic calculator problem in JS.
To implement the change I mentioned above about onclick and code, see @cimmanon's answer, which is much better than what I had here previously.

If you apply a specific class to all buttons that call code, you can get rid of that nasty switch completely.

rezPlz is an interesting variable name ;)

And some nitpicky things:

It's good practice to put semicolons after settings variables and expressions - var code = function() { ... }; is an example of this. However, you don't need a semicolon after explicit function declarations, such as function restring(str) { ... };. Source
Because of your use of onclick, the code won't run if the script is found outside the body. In this case, it's between </body> and </html>, which is pretty weird. Including scripts this way (I believe) also forces the browser to wait until the script is done loading before it displays anything to the user!
I'm not sure if it's because you wanted to include all your code in one code block for this question, but having CSS at the end of the document - especially in <style> tags - is just bizarre. This causes a FOUC! Source

Hopefully this helps!

One thing you can add is that this HTML is missing a doctype, a <head>, a <title> (required for HTML5) and also it has a style completely outside of the closing <html> tag. Other than that, +10! — Ismael Miguel, Aug 12, 2015 at 1:53
this is beautiful answer,thanks for the time you had to waste :p the doctype and the css are messy because I just wanted to get it working, not really a production app :D And for the rest of the pointers, I will take my time with them tomorrow, when I get some sleep. Once again thank you for all this! both of you :) — Bellator, Aug 12, 2015 at 2:13
Actually, you "can't" add a title: in HTML5 you must have a title. And it must have something there (other than a single whitespace). The shortest HTML5 document that validates is <!DOCTYPE html><title>x</title>. Other than that, it looks great! — Ismael Miguel, Aug 12, 2015 at 8:15
@Bellator You're welcome. Scott made most of my work. I was going to address the points on the first comment in an answer, but he said almost everything I was going to. So, the right thing would be to just hand over my knowledge — Ismael Miguel, Aug 12, 2015 at 8:16

Bellator · Accepted Answer · 2015-08-12 15:16:41Z

ok, so I have a solution, I am still not happy with all the ifs. I left button class so new types of buttons can be introduced without big effort. now there is only one action listener and it categorizes buttons in 2 groups, i1 and i2 and they have different behavior. I guess its still not perfect but I am happier with this one!

<!doctype>
<html>

<head>
    <title>calcac</title>
    <meta charset="UTF-8">
    <style rel="stylesheet" type="text/css">
        button {
            font-size: 40px;
            background-color: #FAFAFA;
            min-width: 30px;
        }

        .cont {
            margin: 5px;
            padding-top: 6px
        }

        .b-1{
            padding: 0 5px 0 5px;
            margin-top: 0px;
            border: solid;
            border-width: 1px;
            border-color: blue;
        }

        .b-1:hover{
            background-color: darkblue;
            color: #F0F0F0;
        }

        .col-2 {
            margin-left: 50px;
        }

        .padded {
            padding-right: 25px;
            padding-left: 25px;
        }

        .bordered {
            border: solid;
            border-width: 2px;
            border-color: blue;
            outline: solid;
            outline-width: 1px;
            outline-color: aqua;
        }
    </style>
</head>

<body style="font-size: 40px;  ">
    <div style=" max-width: 400px;" class="bordered">
        <div class="cont bordered">
            <label id="screen">welcome!</label>
        </div>
        <fieldset id="buttons">
            <div class="cont">
                <button class="b-1" id="i1">7</button>
                <button class="b-1" id="i1">8</button>
                <button class="b-1" id="i1">9</button>
                <button class="b-1 col-2" id="i2">+</button>
                <button class="b-1" id="i2">-</button>
            </div>
            <div class="cont ">
                <button class="b-1" id="i1">4</button>
                <button class="b-1" id="i1">5</button>
                <button class="b-1" id="i1">6</button>
                <button class="b-1 col-2" id="i2">*</button>
                <button class="b-1" id="i2">/</button>
            </div>
            <div class="cont">
                <button class="b-1" id="i1">1</button>
                <button class="b-1" id="i1">2</button>
                <button class="b-1" id="i1">3</button>
                <button class="b-1 col-2" id="i2">%</button>
                <button class="b-1" id="i1">(</button>
                <button class="b-1" id="i1">)</button>
            </div>
            <div class="cont ">
                <button class="b-1 padded" id="i1">0</button>
                <button class="b-1 " id="i1">.</button>

                <button class="b-1 col-2 padded" id="i2">=</button>
                <button class="b-1" id="i2">C</button>
            </div>
        </fieldset>
    </div>
</body>

</html>

<script>
    var rg = new RegExp(/\d/);
    var codestr = "";
    var sstring = "";
    //ataching event listener to button pad section
    document.getElementById('buttons').addEventListener('click', calcEvent);
    //event listener logic
    function calcEvent(e) {
        var element = e.target.innerHTML;
        var elementClass = e.target.id;
        sstring = (codestr.substring(codestr.length - 1));
        //filtering input
        if (elementClass == 'i1') {
            code(element);
        } else if (elementClass == 'i2') {

            if (element == "=") {
                compiler();
            } else if (element == "C") {
                clr();
                return;
            } else if (sstring == "(" || sstring == ")" || rg.test(sstring)) {
                code(element);
            } else {
                restring(element);
            }
        }
        document.getElementById('screen').innerHTML = codestr;
    } //event linstener

    //update string functions
    function code(str) {
        if (str === '.' && !rg.test(sstring)) {
            codestr = codestr + '0' + str;
        } else {
            codestr = codestr + str;
        }
    }

    function restring(str) {
        codestr = (codestr.substring(0, codestr.length - 1)) + str
    } //update string

    // results and clear functions
    function compiler() {
        var rezPlz = new Function('return ' + codestr)();
        codestr = "" + rezPlz;
        document.getElementById('screen').innerHTML = codestr;
    }

    function clr() {
        codestr = "";
        document.getElementById('screen').innerHTML = "0";
    } //result & clear
</script>

If you want a further review of your new code, open a new question with it. One thing: ids in a HTML document must be unique. — RoToRa, Aug 19, 2015 at 10:59

AWolf · Accepted Answer · 2015-08-18 20:00:56Z

I have some points for your updated code that could be changed/improved:

CSS You don't need to add the button class b-1 to every button in your markup. It's better to add the class to the parent container and address the button with a css selector >button. Less markup.

HTML IDs of elements have to be unique so it's not recommended adding the id i1 to multiple buttons. Instead add a data-atrribute for your button. e.g. data-type, data-id or everything that makes the intention clear will work.

JS For your if construct you could also use a switch-instruction. That's more readable.

Please have a look at the code below or in this jsfiddle.

var rg = new RegExp(/\d/),
    codestr = "",
    sstring = "";

//ataching event listener to button pad section
document.getElementById('buttons').addEventListener('click', calcEvent);
//event listener logic
function calcEvent(e) {
    var element = e.target.innerHTML,
    	elementClass = e.target.dataset.type,
        braceTest;
    
    sstring = (codestr.substring(codestr.length - 1));
    braceTest = (sstring == "(" || sstring == ")" || rg.test(sstring));
    
    //filtering input
    if (elementClass == 'number') {
        console.log('element added');
        code(element);
    }
    if (elementClass == 'action') {
        switch(element) {
            case '=':
                compiler();
                break;
            case 'DEL':
                codestr = codestr.substring(0, codestr.length - 1);
                break;
            case 'AC':
                clr();
                //return;
                break;
            default:
                if ( braceTest ) {
                    console.log('brace detected or digit');
                    code(element);
                }
                else {
                    restring(element);
                    console.log(codestr, 'rest', element);

                }
        }
    }
    
    document.getElementById('screen').innerHTML = codestr;
} //event linstener

//update string functions
function code(str) {
    if (str === '.' && !rg.test(sstring)) {
        codestr = codestr + '0' + str;
    } else {
        codestr = codestr + str;
    }
}

function restring(str) {
    codestr = (codestr.substring(0, codestr.length - 1)) + str
} //update string

// results and clear functions
function compiler() {
    var rezPlz = new Function('return ' + codestr)();
    codestr = "" + rezPlz;
    document.getElementById('screen').innerHTML = codestr;
}

function clr() {
    codestr = "";
    document.getElementById('screen').innerHTML = "0";
} //result & clear

.wrapper {
    max-width: 400px;
}

button {
    font-size: 40px;
    background-color: #FAFAFA;
    min-width: 30px;
}
.cont {
    margin: 5px;
    padding-top: 6px
}
.b-1>button {
    padding: 0 5px 0 5px;
    margin-top: 0px;
    border: solid;
    border-width: 1px;
    border-color: blue;
}
.b-1>button:hover {
    background-color: darkblue;
    color: #F0F0F0;
}
.col-2 {
    margin-left: 50px;
}
.padded {
    padding-right: 25px;
    padding-left: 25px;
}
.bordered {
    border: solid;
    border-width: 2px;
    border-color: blue;
    outline: solid;
    outline-width: 1px;
    outline-color: aqua;
}

<div class="wrapper bordered">
    <div class="cont bordered">
        <label id="screen">welcome!</label>
    </div>
    <fieldset id="buttons">
        <div class="cont b-1">
            <button data-type="number">7</button> <!-- ids have to be unique -->
            <button data-type="number">8</button>
            <button data-type="number">9</button>
            <button class="col-2" data-type="action">+</button>
            <button data-type="action">-</button>
        </div>
        <div class="cont b-1">
            <button data-type="number">4</button>
            <button data-type="number">5</button>
            <button data-type="number">6</button>
            <button class="col-2" data-type="action">*</button>
            <button data-type="action">/</button>
        </div>
        <div class="cont b-1">
            <button data-type="number">1</button>
            <button data-type="number">2</button>
            <button data-type="number">3</button>
            <button class="col-2" data-type="action">%</button>
            <button data-type="number">(</button>
            <button data-type="number">)</button>
        </div>
        <div class="cont b-1">
            <button class="padded" data-type="number">0</button>
            <button data-type="number">.</button>
            <button class="col-2 padded" data-type="action">=</button>
            <button data-type="action">DEL</button>
            <button data-type="action">AC</button>
        </div>
    </fieldset>
</div>

\$\begingroup\$ It looks much cleaner this way, thanks for the tip on ids! \$\endgroup\$
– Bellator
Aug 19, 2015 at 9:31 — Bellator, Aug 19, 2015 at 9:31

Stack Exchange Network

Basic web calculator

4 Answers 4

Event Delegation

Unsemantic class names

HTML

Javascript

Your Answer

Not the answer you're looking for? Browse other questions tagged
javascript
html
css
calculator
or ask your own question.

Hot Network Questions

Basic web calculator

4 Answers 4

Event Delegation

Unsemantic class names

HTML

Javascript

Your Answer

Sign up or log in

Post as a guest

Not the answer you're looking for? Browse other questions tagged javascripthtmlcsscalculator or ask your own question.

Related

Hot Network Questions

Not the answer you're looking for? Browse other questions tagged
javascript
html
css
calculator
or ask your own question.