How to get a right value in the result? (calculator js)

Question

Good day. I have a mistake in the logic of storing the numbers, but i can't figure out why. In result, i need to have the correct answer like "1+1 = 2", instead i have "1+1 = 1". And one more question is how to store a lot of operations, like "2 + 2 / 4 * 1"?

I will be very appreciate for your help.

p.s. https://codepen.io/Dmxvil/pen/MWyBqOz (you can see the calculator here with html)

const numbers = document.querySelectorAll('[data-calc]'),
    operators = document.querySelectorAll('[data-operator]'),
    prev = document.querySelector('.prev-result'),
    current = document.querySelector('.current-number'),
    //resetBtn = document.querySelector('.operator-btn-reset'),
    resultBtn = document.querySelector('[data-result]');

let prevData = [];
let currentData = [];
let operation,
computation;

    numbers.forEach((number => {
        number.addEventListener('click', () => {
            current.innerHTML = number.innerText;
            currentData.push(number.dataset.calc);
            current.innerHTML = currentData.join('');
        });
    }));


function updateDisplay(){
        prev.innerHTML = currentData.join('');
        prevData = prev.innerHTML;
        currentData.splice(0, 100);
        current.innerHTML = '';
}


function chooseOperator() {
    operators.forEach(operator => {
        operator.addEventListener('click', () => {
            operation = operator.innerText;
            switch (operator.innerText) {
                case '+':
                    console.log(operation);
                    computation = prevData + currentData;
                    console.log(computation);
                    break;
                case '-':
                    console.log(operation);
                    computation = prevData - currentData;
                    break;
                case '*':
                    console.log(operation);
                    computation = prevData * currentData;
                    break;
                case '/':
                    console.log(operation);
                    computation = prevData / currentData;
                    break;
            }

            updateDisplay();
        });
    });
}

chooseOperator();

function result(){
    prev.innerHTML = prevData + operation + currentData.join('');
    current.innerHTML = computation.toString();
}

resultBtn.addEventListener('click', ()=> {
    result();
});

Good that you are learning JS. Can you add HTML code also, so your snippet will be working? It is interesting to look at working ("almost") example — Anton, Sep 16 '20 at 08:38
JavaScript does not care about types of the variables. Sometimes it is good, but most of the time it is bad. For example you have variable: `let prevData = [];` (so it is array), but then `prevData = prev.innerHTML;` (oh, come on! you replace array with a string, so now array is lost). In that case: `prevData` must be array or string? :) — Anton, Sep 16 '20 at 08:42
@Anton i add a pen https://codepen.io/Dmxvil/pen/MWyBqOz , thank you. i think i need to save my array but transform it in string to show it in display? — , Sep 16 '20 at 08:46
You are about to write your own simple parser to execute strings like `2 + 2 / 4 * 1`. It might be tricky depending on the complexity of the expressions. Or alternatively have a look at `eval` function, which of course has its pros and cons as well - https://javascript.info/eval When is JavaScript's eval() not evil? - https://stackoverflow.com/questions/197769/when-is-javascripts-eval-not-evil — Ihor Vyspiansky, Sep 16 '20 at 08:52

score 0 · Answer 1 · answered Sep 16 '20 at 09:16

Something like that? I used eval.

const numbersAndOperators = document.querySelectorAll('[data-calc]'),
  prev = document.getElementById('prev-result'),
  current = document.getElementById('current-number'),
  resultBtn = document.querySelector('[data-result]'),
  acButton = document.querySelector('[data-operator=AC]');

numbersAndOperators.forEach((button => {
  button.addEventListener('click', () => {
    current.innerHTML += button.innerText;
  });
}));

acButton.addEventListener('click', () => {
  current.innerHTML = '';
});

function result() {
  try {
    let res = eval(current.innerHTML);
    prev.innerHTML = current.innerHTML + ' = ' + (isNaN(res) ? 'Undefined' : res);
    current.innerHTML = '';
  } catch (e) {
    prev.innerHTML = e.message;
    console.log(e.message);
  }
}

resultBtn.addEventListener('click', result);

* {
  box-sizing: border-box;
  margin: 0;
  padding: 0;
}

.calculator .calculator-grid {
  width: 300px;
  height: 550px;
  margin-top: 50px;
  background-color: white;
  border: 1px solid lightgrey;
  border-radius: 30px;
  padding: 10px;
}

.calculator {
  display: flex;
  flex-direction: column;
  justify-content: center;
  align-items: center;
}

.calculator .calculator__buttons {
  display: grid;
  width: 30px;
  grid-template-columns: repeat(4, 1fr);
}

.calculator .calculator__buttons button {
  margin: 5px;
  width: 60px;
  height: 60px;
  border-radius: 50%;
  border: none;
  background-color: gainsboro;
  color: black;
  font-size: 30px;
}

.calculator .calculator__buttons button:active {
  background-color: darkgray;
}

.calculator .calculator__input .calc-input {
  margin: 15px 0;
  padding: 10px;
  width: 280px;
  height: 100px;
  border: none;
  background-color: white;
  font-size: 40px;
  color: black;
  text-align: right;
}

.calculator .calculator__input .calc-input:active {
  border: none;
}

.calculator .calculator__input .calc-input .prev-result {
  height: 40px;
  font-size: 30px;
  overflow: hidden;
  white-space: nowrap;
}

.calculator .calculator__input .calc-input .current-number {
  height: 60px;
}

.calculator .calculator__buttons .operator-btn {
  font-size: 30px;
  color: white;
  background-color: darkturquoise;
}

.calculator .calculator__buttons .operator-btn:active {
  background-color: aqua;
}

<section class="calculator">
  <div class="calculator-grid">
    <div class="calculator__input">
      <div class="calc-input">
        <div class="prev-result" id="prev-result"></div>
        <div class="current-number" id="current-number"></div>
      </div>
    </div>
    <div class="calculator__buttons">
      <button data-calc="7">7</button>
      <button data-calc="8">8</button>
      <button data-calc="9">9</button>
      <button data-operator="-" data-calc="-" class="operator-btn-minus operator-btn">-</button>
      <button data-calc="4">4</button>
      <button data-calc="5">5</button>
      <button data-calc="6">6</button>
      <button data-operator="+" data-calc="+" class="operator-btn-plus operator-btn">+</button>
      <button data-calc="3">3</button>
      <button data-calc="2">2</button>
      <button data-calc="1">1</button>
      <button data-operator="*" data-calc="*" class="operator-btn operator-btn-multiply">*</button>
      <button data-calc="0">0</button>
      <button data-operator="AC" class="operator-btn operator-btn-reset">AC</button>
      <button data-result="=" class="operator-btn operator-btn-result">=</button>
      <button data-operator="/" data-calc="/" class="operator-btn operator-btn-divide">/</button>
    </div>
  </div>
</section>

P.S. As I use eval, so you can also add brackets ( and ) here. For math operations like sqrt or log we need some other technique...

Thank you! Yes it's works like i need, but i'm trying to avoid eval(), so i need to keep switch or write a separate functions — , Sep 16 '20 at 09:33
it's my task in the course i'm studying ( https://www.theodinproject.com/courses/web-development-101/lessons/calculator ) — , Sep 16 '20 at 09:55
@vol4ikeurope I'll look at your question later if no-one will answer before me =) — Anton, Sep 16 '20 at 10:10

score 0 · Accepted Answer · answered Sep 16 '20 at 17:14

Take a look. Without 'eval'. Maybe, not optimal in some ways, but at least it is working as expected. It means that 12 + 7 - 5 * 3 will be 42.

I've added comments to the code, but you can ask if something is not clear.

Note: I also added two new labels - one to show the operator and another one to show the expression. Why? Because it is more beautiful for me. If you do not need them - just say. We can remove them.

const numbers = document.querySelectorAll('[data-calc]'),
  operators = document.querySelectorAll('[data-operator]:not([data-operator=AC])'),
  prev = document.getElementById('prev-result'),
  current = document.getElementById('current-number'),
  resultBtn = document.querySelector('[data-result]'),
  acButton = document.querySelector('[data-operator=AC]'),
  operatorLabel = document.getElementById('operator'),
  expression = document.getElementById('expression');

let firstNumber; // this is previous result
let secondNumber; // this is what we enter with keyboard

const setOperator = (x) => operatorLabel.innerHTML = x !== undefined ? x : '';
const getOperator = () => operatorLabel.innerHTML;

numbers.forEach(button => {
  button.addEventListener('click', () => {
    if (firstNumber !== undefined && getOperator() == '') {
      // ignore previous result if we did not link it with operator
      firstNumber = undefined;
    }
    if (isNaN(parseInt(current.innerHTML))) current.innerHTML = '';
    current.innerHTML += button.innerText;
    secondNumber = parseInt(current.innerHTML);
  });
});

operators.forEach(button => {
  button.addEventListener('click', () => {
    if (secondNumber !== undefined) {
      if (firstNumber !== undefined) {
        // continue of expression
        const result = processResult();
        setOperator(button.innerText);
        firstNumber = result.result;
      } else {
        // we have no previous result
        firstNumber = secondNumber;
        secondNumber = undefined;
        current.innerHTML = '';
        prev.innerHTML = firstNumber;
        setOperator(button.innerText);
      }
    } else if (firstNumber !== undefined) {
      // we have previous result, but empty next number
      setOperator(button.innerText);
    }
  });
});

acButton.addEventListener('click', () => {
  current.innerHTML = '';
  secondNumber = undefined;
  setOperator();
});

const getResult = () => {
  const operator = getOperator();
  if (firstNumber !== undefined && secondNumber !== undefined && operator != '') {
    const str = String(firstNumber) + operator + secondNumber;
    let result = {
      result: NaN,
      str
    };
    if (operator == '+') result.result = firstNumber + secondNumber;
    if (operator == '-') result.result = firstNumber - secondNumber;
    if (operator == '/') result.result = firstNumber / secondNumber;
    if (operator == '*') result.result = firstNumber * secondNumber;
    return result;
  }
}

const processResult = () => {
  const result = getResult();
  if (result !== undefined) {
    if (isNaN(result.result)) {
      current.innerHTML = 'Error';
      setOperator();
      prev.innerHTML = '';
    } else {
      prev.innerHTML = result.result;
      firstNumber = result.result;
      current.innerHTML = '';
      secondNumber = undefined;
      setOperator();
    }
    expression.innerHTML = result.str + ' = ' + (isNaN(result.result) ? 'Undefined' : result.result);
  }
  return result;
}

resultBtn.addEventListener('click', () => {
  processResult();
});

* {
  box-sizing: border-box;
  margin: 0;
  padding: 0;
}

.calculator .calculator-grid {
  width: 300px;
  height: 550px;
  margin-top: 50px;
  background-color: white;
  border: 1px solid lightgrey;
  border-radius: 30px;
  padding: 10px;
}

.calculator {
  display: flex;
  flex-direction: column;
  justify-content: center;
  align-items: center;
}

.calculator .calculator__buttons {
  display: grid;
  width: 30px;
  grid-template-columns: repeat(4, 1fr);
}

.calculator .calculator__buttons button {
  margin: 5px;
  width: 60px;
  height: 60px;
  border-radius: 50%;
  border: none;
  background-color: gainsboro;
  color: black;
  font-size: 30px;
}

.calculator .calculator__buttons button:active {
  background-color: darkgray;
}

.calculator .calculator__input .calc-input {
  margin: 0 0 15px 0;
  padding: 10px;
  width: 280px;
  height: 100px;
  border: none;
  background-color: white;
  font-size: 40px;
  color: black;
  text-align: right;
}

.calculator .calculator__input .calc-input:active {
  border: none;
}

.calculator .calculator__input .calc-input .prev-result {
  height: 40px;
  font-size: 30px;
  overflow: hidden;
  white-space: nowrap;
}

.calculator .calculator__input .calc-input .current-number {
  height: 60px;
}

.calculator .calculator__buttons .operator-btn {
  font-size: 30px;
  color: white;
  background-color: darkturquoise;
}

.calculator .calculator__buttons .operator-btn:active {
  background-color: aqua;
}

div.prev-container {
  position: relative;
}

div.operator {
  position: absolute;
  top: 25px;
  right: -10px;
  font-size: 30px;
}

#expression {
  position: absolute;
  left: 4px;
  right: 4px;
  text-align: left;
  top: -16px;
  font-size: 14px;
  overflow: hidden;
  white-space: nowrap;
}

<section class="calculator">
  <div class="calculator-grid">
    <div class="calculator__input">
      <div class="calc-input">
        <div class="prev-container">
          <div id="expression"></div>
          <div class="prev-result" id="prev-result"></div>
          <div class="operator" id="operator"></div>
        </div>
        <div class="current-number" id="current-number"></div>
      </div>
    </div>
    <div class="calculator__buttons">
      <button data-calc="7">7</button>
      <button data-calc="8">8</button>
      <button data-calc="9">9</button>
      <button data-operator="-" class="operator-btn-minus operator-btn">-</button>
      <button data-calc="4">4</button>
      <button data-calc="5">5</button>
      <button data-calc="6">6</button>
      <button data-operator="+" class="operator-btn-plus operator-btn">+</button>
      <button data-calc="3">3</button>
      <button data-calc="2">2</button>
      <button data-calc="1">1</button>
      <button data-operator="*" class="operator-btn operator-btn-multiply">*</button>
      <button data-calc="0">0</button>
      <button data-operator="AC" class="operator-btn operator-btn-reset">AC</button>
      <button data-result="=" class="operator-btn operator-btn-result">=</button>
      <button data-operator="/" class="operator-btn operator-btn-divide">/</button>
    </div>
  </div>
</section>

thank you i'm exploring your code and try to change mine. here what i have done yesterday by myself https://codepen.io/Dmxvil/pen/MWyBqOz , only need to do smth with inputs because it shows only one number. what can you say about this solution? — , Sep 17 '20 at 09:42
@vol4ikeurope just a quick look. 1) `takePrevValue` name of the function is not correct. It does not take value, it just adds event listeners, 2) same for `clear` and `chooseOperator`, `takeCurrentValue`, 3) `takePrevValue` and `takeCurrentValue` append duplicate event, you should combine this 2 event listeners to one — Anton, Sep 17 '20 at 10:24
@vol4ikeurope this line `if (prevData === '' && currentData === '')` blocks every input beyond one symbol — Anton, Sep 17 '20 at 10:25
@vol4ikeurope `currentData.toString()` - in your code `currentData` is always the string. No need to convert it again, right? Same for `num.toString()`, because `num` is already a string — Anton, Sep 17 '20 at 10:26
i replaced takePrevValue and takeCurrentValue by a single function appendNum, clear by reset, chooseOperator by getOperator and also deleted unnecessary converts. thank you for review. trying to figure out what is the problem with conditions of input — , Sep 17 '20 at 11:57

How to get a right value in the result? (calculator js)

2 Answers2