Virge Assault
Virge Assault

Reputation: 1396

Javascript firing function twice and 1 of 4 functions not working

I'm creating a calculator in JavaScript. It is working, except for the add() function (sub, mult, and div work fine) and when I press "=" it sends the answer twice in back to back pop-ups.

Add returns the value as if both x and y were strings, i.e. 7+8 = 78. How can I get it to calculate add, and to send the answer just once?

calculator.html

<!DOCTYPE html>
<html>
<head><title>Calculator</title></head>
<link rel="stylesheet" type="text/css" href="calc.css"/>
<body>

<p>
    <label for="x">First Number</label>
    <input type="text" name="x" id="x" />
</p>
<p>
    <label for="y">Second Number:</label>
    <input type="text" name="y" id="y" />
</p>

<p>
    <input type=radio name="Operation" value=Add id="0" /> <label for="0">Add</label>
    <input type=radio name="Operation" value=Sub id="1" /> <label for="1">Sub</label>
    <input type=radio name="Operation" value=Mult id="2" /> <label for="2">Mult</label>
    <input type=radio name="Operation" value=Div id="3" /> <label for="3">Div</label>
</p>
<p>
    <input type="button" value="=" id="equals"/>
</p>

<script type="text/javascript" src="calculator2.js"></script>

</body>
</html> 

calculator2.js

var x;
var y;
var sign;
var answer;
var equals = document.getElementById("equals");

document.getElementById("0").addEventListener("click", getSign, false);
document.getElementById("1").addEventListener("click", getSign, false);
document.getElementById("2").addEventListener("click", getSign, false);
document.getElementById("3").addEventListener("click", getSign, false);
document.getElementById("equals").addEventListener("click", Calculator, false);

equals.onclick = function(){
    Calculator();
}

function getSign(event) {
    sign = event.target.id;
}

function add(){
    answer = x+y;
}

function sub(){
    answer = x-y;
}

function mult(){
    answer = x*y;
}

function div(){
    if(y==0){
        alert("Cannot divide by zero");
        return;
    }
    else{
        answer = x/y
    }
}

function Calculator(){
    x = document.getElementById("x").value;
    y = document.getElementById("y").value;


    if (sign == 0){
        add();
    }
    else if (sign == 1){
        sub();
    }
    else if (sign == 2){
        mult();
    }
    else {
        div();
    }

    alert(answer);
}

Upvotes: 1

Views: 290

Answers (4)

user3459110
user3459110

Reputation: 7061

Why the add() function won't work?

The value of the the input elements has a String type. Using the + operator on them will add them as string only. To make it work as you intend, you cast those strings to Numbers via the Number method.

var myString = "1";
var castingToNumber = Number(myString);
console.log(myString, castingToNumber); //logs "1" and 1 (notice the quotes)

The reason why this doesn't happen with the - operator is that strings can't be subtracted, and thus Javascript automatically parses those strings to Numbers and then operates on them. This should NOT be relied upon.

Two Alerts?

As other answers point out, you have two event listeners. It is recommended that you remove the second event listener, and keep the one defined via addEventListener. This allows you to add more event listeners later, without losing the original ones.

Upvotes: 2

Midhun Murali
Midhun Murali

Reputation: 2151

You have 2 event handlers attached to the clik event of '='(equals) button .Removing event handler one will give you only one alert with answer

document.getElementById("equals").addEventListener("click", Calculator, false);

equals.onclick = function(){
    Calculator();
}

From Calculator.js remove

equals.onclick = function(){
        Calculator();
    }

Also in function add multiply the value with * 1 to consider the input as number's

function add(){
    answer = (x*1)+(y*1);
}

This will give you correct sum

Working Fiddle at http://jsfiddle.net/Midhun52/sskatgta/1/

Upvotes: 1

dfsq
dfsq

Reputation: 193301

Since your calculator is expected to work with numbers you should cast strings (you get from input field) to number type:

x = Number(document.getElementById("x").value);
y = Number(document.getElementById("y").value);

Of course you could also change add function

function add() {
    answer = Number(x) + Number(y);
}

but this is more like workaround. Arithmetic operations should be done on numbers in first place.

Upvotes: 1

user1594816
user1594816

Reputation: 3

You are defining 2 event handlers, so this is why code Calculator is called twice.

document.getElementById("equals").addEventListener("click", Calculator, false);

equals.onclick = function(){
    Calculator();
}

Upvotes: 0

Related Questions