Calculator giving wrong results in JS

Question

i'm learning some javascript and i tried to make a simple calculator.

The sum worked just fine, but the rest of the operations didnt work. What i'm doing wrong?

js:

function soma(){
    var s1 = parseInt(document.getElementById("valor1").value,10);
    var s2 = parseInt(document.getElementById("valor2").value,10);
    document.getElementById("resultado").style.display = "block";

    if(document.getElementById("btnSoma").onclick){
        var soma = s1 + s2;
        document.getElementById("resultado").innerHTML = soma;
    }else if(document.getElementById("btnSubtrair").onclick){
        var subtrair = s1 - s2;
        document.getElementById("resultado").innerHTML = subtrair;
    }else if(document.getElementById("btnMultiplicar").onclick){
        var mult = s1 * s2;
        document.getElementById("resultado").innerHTML = mult;
    }else if(document.getElementById("btnDividir").onclick){
        var div = s1 / s2;
        document.getElementById("resultado").innerHTML = div;
    }


}

JSFiddle link: https://jsfiddle.net/y7r1m4nx/ with HTML

Also, can someone please explain why i have to put the ,10 in parseInt? i didnt understand it very well from Mozilla docs.

Answer 1

Here is the code working:

function soma(type){
  var s1 = parseInt(document.getElementById("valor1").value,10);
  var s2 = parseInt(document.getElementById("valor2").value,10);
  document.getElementById("resultado").style.display = "block";
  var result;

  if (type === '+') {
    result = s1 + s2;
  } else if (type === '-') {
    result = s1 - s2;
  } else if(type === '*'){
    result = s1 * s2;
  }else if(type === '/'){
    result = s1 / s2;
  }

  document.getElementById("resultado").innerHTML = result;
}

<div class="container text-center">
  <input name="Valor 1" id="valor1">
  <input name="Valor 2" id="valor2"><br>
  <button type="submit" onclick="soma('+')" id="btnSoma">Somar</button><br>
  <button type="submit" onclick="soma('-')" id="btnSubtrair">Subtrair</button><br>
  <button type="submit" onclick="soma('/')" id="btnDividir">Dividir</button><br>
  <button type="submit" onclick="soma('*')" id="btnMultiplicar">Multiplicar</button><br>
  <p id="resultado">Resultado</p>
</div>

You can't look at the element to see which was clicked.

if(document.getElementById("btnSoma").onclick)

Instead you can pass in some value you can use to determine the action. In teh example code I am passing in a parameter to indicate the action I want to happen:

Add:'+', Subract:'-', Multiply:'*' and divide:'/'

There are other ways to accomplish this, including having each onclick call a different function.

But this should help.

Answer 2

Well, your if statement is incorrect. You are actually only checking if onClick was defined on your button that you retrieve.

As it has been defined, you will always end up in the first if-block, so you will never really do something else but sums. It is just an eventhandler to indicate what the program would do when you actually click it, which you forward to the soma() function.

Now, I don't speak the language you seem to be speaking, so having this question be in English, would be helpful, but I am thinking that soma means sum?

If that is the case, you could make as a starting point 4 different functions, that handle the same input, but perform different operations, something like

// get the element once, the values you can just retrieve when needed
const value1 = document.getElementById('value1');
const value2 = document.getElementById('value2');
const output = document.getElementById('result');

function getValuesFromInput() {
  return {
    a: parseInt( value1.value ),
    b: parseInt( value2.value )
  };
}

function setOutput( result ) {
  output.innerText = result;
}

function sum() {
  const { a, b } = getValuesFromInput();
  setOutput( a + b );
}

function subtract() {
  const { a, b } = getValuesFromInput();
  setOutput( a - b );
}

function multiply() {
  const { a, b } = getValuesFromInput();
  setOutput( a * b );
}

function divide() {
  const { a, b } = getValuesFromInput();
  setOutput( a / b );
}

<input id="value1">
<input id="value2"><br>
<button type="button" onclick="sum()">Sum</button><br>
<button type="button" onclick="subtract()">Subtract</button><br>
<button type="button" onclick="multiply()">Multiply</button><br>
<button type="button" onclick="divide()">Divide</button><br>
<span id="result"></span>

This would be the easiest version to transform in. Nowadays, people really don't use the onclick syntax inside html anymore, so you would now rather attach an event inside your javascript, and let it get handled there, for example

// get the element once, the values you can just retrieve when needed
const value1 = document.getElementById('value1');
const value2 = document.getElementById('value2');
const output = document.getElementById('result');

document.getElementById('btnSum').addEventListener('click', sum);
document.getElementById('btnSubtract').addEventListener('click', subtract);
document.getElementById('btnDivide').addEventListener('click', divide);
document.getElementById('btnMultiply').addEventListener('click', multiply);

function getValuesFromInput() {
  return {
    a: parseInt( value1.value ),
    b: parseInt( value2.value )
  };
}

function setOutput( result ) {
  output.innerText = result;
}

function sum() {
  const { a, b } = getValuesFromInput();
  setOutput( a + b );
}

function subtract() {
  const { a, b } = getValuesFromInput();
  setOutput( a - b );
}

function multiply() {
  const { a, b } = getValuesFromInput();
  setOutput( a * b );
}

function divide() {
  const { a, b } = getValuesFromInput();
  setOutput( a / b );
}

<input id="value1">
<input id="value2"><br>
<button type="button" id="btnSum">Sum</button><br>
<button type="button" id="btnSubtract">Subtract</button><br>
<button type="button" id="btnMultiply">Multiply</button><br>
<button type="button" id="btnDivide">Divide</button><br>
<span id="result"></span>

This in total doesn't bring a lot, but you could of course change the html slightly to allow for all operators to be handled, and where it finally gets a bit more interesting, because you can finally use the event argument

const value1 = document.getElementById('value1');
const value2 = document.getElementById('value2');
const output = document.getElementById('result');

document.querySelectorAll('[data-operator]').forEach( btn => btn.addEventListener( 'click', calculate ) );

function calculate( eventSource ) {
  const operator = eventSource.target.getAttribute('data-operator');
  if (!operator) {
    return;
  }
  // eval should be used carefully here, but it is just demonstrating what you could get to
  output
    .innerText = eval( `(${parseInt(value1.value)}) ${operator} (${parseInt(value2.value)})` );
}

<input id="value1">
<input id="value2"><br>
<button type="button" data-operator="+">Sum</button><br>
<button type="button" data-operator="-">Subtract</button><br>
<button type="button" data-operator="*">Multiply</button><br>
<button type="button" data-operator="/">Divide</button><br>
<span id="result"></span>

Now, for the parseInt, the second argument would be the base, for example

console.log( parseInt( '11', 2 ) ); // input is in binairy format
console.log( parseInt( '11', 10 ) );// input is in decimal format
console.log( parseInt( '11', 16 ) );// input is in hexadecimal format

Answer 3

You should set up your javascript where

document.getElementById("btnSoma").click(function() {
  var add = s1 + s2;
  document.getElementById("resultado").innerHTML = add;
});

and repeat this for mult, div, subtraction as desired. You do not need to call soma() as an onclick function with this implementation.

Also, the javascript parseInt function can take two parameters, the value to parse and the radix which is what base you are processing numbers in. Typically we count in base 10, but some programs may use different bases.