Help with JS Calculator?

Hey, guys i’m having a hard time figuring out how start a new equation after pressing ‘=’ on my calculator. For example, if you do 8+5 then press equals it should show 13. Then if I were to press another number a new equation should start but instead I get 138 on the screen. I’ve tried storing the previous answer in an array but I couldn’t figure out where to insert the previous answer in my code. I would appreciate any help, thanks!

Hi mish,

I have tried to replicate the problem of your code on my computer and I couldn’t; instead I was facing a different one.

  1. on Netlify, I could see the behaviour you describe.
  2. on my Mac using Safari and Chrome (89), I faced the problem that I could only enter 1-digit-numbers

Looking at your code, it is quite difficult to maintain, I’d say. So I am proposing some changes and I am sending you a working model (where not all is implemented yet) back.

This super-long button function is really quite something to understand!

Instead, I have suggested that you create separate event listeners for the different kinds of buttons and implement the functionality independently. You can keep track of your data using the object calculation (or by using individual variables; this is just a bit more elegant and you can copy it to an archive if you want to keep track of a history of values).

Happy to explain my code more.

Keep up! :slight_smile:
Best, Sebastian.

I forked your files, see my suggestions there. I couldn’t contribute to your code as I didn’t want to upset the master and there was no dev branch available.

1 Like

As a general suggestion, you want to keep track of states. First you’re in initial state (or start state). In this state, when a digit button is pressed, you enter into the “waiting for the left operand” state, in which you keep adding digits to the display. When any of the operator button is pressed, you pushed the current display value to the value stack and the operator to the operator stack. You’re now in the “waiting for the second operation” state for entering the second operand. In this state, when the digit buttons are pressed, you keep adding them to the display. Now, when the equal button is pressed, you compute the operation and display the result and go back to the start state.
You design a handful of states, and for each state, you determine what to do for different button press and whether to transition to another state.

Thank you for sharing me your answer. I went over it and it’s great! But now it’s stuck in my head…I need to think of how I can improve mine without feeling like I’m copying yours :slight_smile:

Thanks, mish!

Good luck on your coding journey!

What really helped me was a guy telling me that it’s totally ok to write longer code if it becomes much more readable. I’d like to honor him and share that with you!

I can suggest some more approaches. In particular, @twotani made a sensible suggestion which you could probably implement using the MVC-pattern (which will also add another layer of practical experience!) :slight_smile: Alternatively, why not take the calculation as an object which is instantiated with a class that holds all the relevant functions?

All the best,
Sebastian.

Thanks Sebastian!

I started fixing my code. I thought to add an event listener to every kind of button and then add the if -else statements to each button… not sure if this still consider bad practice but I think it’s more readable…I will still look into the other suggestions as my method might turn out as another mess :sweat_smile: I’ve been learning for almost 2 years and still need a lot of help!

Anyway, thanks for everything. I really appreciate it :+1:

let input = document.querySelector('#userInput');
let operatorBtn = document.querySelectorAll('.btn-operator');
let numberBtn = document.querySelectorAll('.btn-number');
let otherBtn = document.querySelectorAll('.btn-other');
let firstNum = '';
let secondNum = '';
let operator = '';
let total = '';
let hint = document.querySelector('.hint');

numberBtn.forEach(function (btn) {
    btn.addEventListener('click', function () {
        if (operator == "" && !firstNum.includes('%')) {
            firstNum += btn.value;
            input.value = firstNum;
        }
        else {
            if (!secondNum.includes('%')) {
                secondNum += btn.value;
                input.value = secondNum;
            }
        }
        console.log(firstNum);
        console.log(secondNum);
    });
});

operatorBtn.forEach(function (btn) {
    btn.addEventListener('click', function () {
        operator += btn.value;
        console.log(operator);
    });
});

otherBtn.forEach(function (btn) {
    btn.addEventListener('click', function () {
        if (firstNum != "" && operator == "") {
            if (firstNum.includes('.') && btn.value == ('.')) {
                btn.disabled = true;
            } else if (firstNum.includes('%') && btn.value == ('%')) {
                btn.disabled = true;
            } else {
                firstNum += btn.value;
                input.value = firstNum;
            }
            btn.disabled = false;
        }
        else {
            if (secondNum.includes('.') && btn.value == ('.')) {
                btn.disabled = true;
            } else if (secondNum.includes('%') && btn.value == ('%')) {
                btn.disabled = true;
            } else {
                secondNum += btn.value;
                input.value = secondNum;
            }
            btn.disabled = false;
        }
    });
});

document.querySelector(".clear-btn").addEventListener("click", function () {
    firstNum = '';
    secondNum = '';
    operator = '';
    input.value = '';
});

document.querySelector(".btn-back").addEventListener("click", function () {
    if (firstNum.length > 0 && operator == '' && secondNum.length == 0) {
        input.value = input.value.substring(0, input.value.length - 1);
        firstNum = input.value;
        console.log(firstNum);
    } else if (firstNum.length > 0 && operator != '' && secondNum.length == 0) {
        operator = '';
        input.value = firstNum;
        console.log(operator);
    } else if (firstNum.length > 0 && operator != '' && secondNum.length > 0) {
        input.value = input.value.substring(0, input.value.length - 1);
        secondNum = input.value;
        console.log(secondNum);
        if (secondNum.length == 0) {
            input.value = operator;
        }
    } else {
        hint.style.display = 'block';
        setTimeout(function () { hint.style.display = 'none' }, 2000);
        firstNum = '';
        secondNum = '';
        operator = '';
        input.value = '';
    }
});
1 Like

Here’s a detailed follow up post. Given a problem, there are general approaches or techniques you can use to solve. Decision trees, flowcharts, truth tables, Venn diagrams, state machines, etc. etc. I would like to discuss how the state machine can be helpful for the calculator application. I used a state machine to implement this application. I maintain five states as such

const State = {
  ST: 0,    //start state
  LT: 1,    //handling left operand
  OP: 2,    //operator entered
  RT: 3,    //handling right operand
  EQ: 4     //equal entered
}

Using a state machine will eliminate the needs to include all kinds of if tests, checking for various situations, inside the code. A state machine approach makes the code clean, easy to read, and easy to maintain / modify / extend. It allows to handle the program logic in a consistent manner.

I define a function enterDigit() to handle the pressing of a button 1 ~ 9. Here’s the element for 7.

<button type="button" value="7" id="seven" onclick="enterDigit('7')">7</button>

The function enterDigit(d) is defined as

const enterDigit = (digit) => {
  switch (calculator.state) {
    case State.ST:
    case State.OP: 
    case State.EQ: calculator.display = digit;
                   break;
    case State.LT:
    case State.RT: calculator.display += digit;
                   break;
  } 

  changeState(calculator.state, Move.DG);
  updateDisplay();
}

My calculator object is this

const calculator = {
  state: State.ST,
  display: S_ZERO,  //"0"
  hasDecimal: false,
  isNeg: false,
  leftOp: ZERO,   //0.0
  rightOp: ZERO,
  op: NONE   //""
}

If the calculator state is ST, OP, or EQ, then it means the digit is the first digit of an operand, so I set

calculator.display = digit;

If the calculator is in the LT or RT state, then the digit entered is added to the current value

calculator.display += digit;

The calculator is never going to be in LT or RT unless the calculator saw the first digit so I just append the newly entered digit. The state transition is handled by calling

changeState(calculator.state, Move.DG);

In this function, if, for example, the current state is ST and the action Move.DG (for digit), then the state will change to State.LT. If the current state is LT or RT, then there’s no transition, it will remain in the same state ( there’s no matching case in the switch statement in changeState, so nothing happens, meaning the calculator will remain in the same state). There are five different possible moves (transitions from one state to another)


const Move = {
  ZE: 0,    //0 entered
  DG: 1,    //1 - 9 entered
  DT: 2,    // . entered
  OP: 3,    //operator
  EQ: 4     // = entered
}

Analogous to the enterDigit function, I have

enterZero()
enterDot()
enterOp(op)   //to handle +, -, /
enterMinus() //to handle unary or binary minus
enterEq()

All of these functions (except for the unary minus case) will call changeState() with appropriate arguments and updateDisplay(). Here’s a part of enterOp

const enterOp = (op) => {
  switch (calculator.state) {
    case State.LT:
    case State.EQ:
      calculator.leftOp = parseFloat(calculator.display);
      if (calculator.isNeg) calculator.leftOp = -calculator.leftOp;
      calculator.op = operator[op];
      break;
    . . .

When the calculator sees an operator (+, *, or /) and the state is LT or EQ, then it means that we have a value displayed on the calculator that is going to be the left operand, so I convert it to a float value

calculator.leftOp = parseFloat(calculator.display);

I check if a unary minus was entered, and if it was, I negate the value

 if (calculator.isNeg) calculator.leftOp = -calculator.leftOp;

Finally, I store the given operator

calculator.op = operator[op];

The operator object is defined as

const operator = {
  "+" : (left, right) => left + right,
  "-" : (left, right) => left - right,
  "*" : (left, right) => left * right,
  "/" : (left, right) => left / right
}

All other enterXX functions are similarly structured using a switch statement based on state.

Using a state machine really makes the code easy to understand, and thus less error-prone. It also allows easier extension. For instance, if I want to add a back button to clear the last entered digit, I simply add enterBack() function ( (I might add Move.BK to make everything consistent with other enterXX functions). The back button is effective only when the calculator is in LT or RT state and the calculator.display is not S_ZERO. The general structure of the code does not change, and I don’t have to make any changes to the existing code.

The MVC design pattern was mentioned. Exactly what does the term mean is open to discussion, but I understand the general philosophy is to maintain a clear separation of interest. Keep the code for UI and Logic separate and the coupling between the two to the minimum. The only connection from my UI (html) is calling the functions in the script (logic). There’s no reliance on what is being displayed or happening on the UI side from the script. The script is in full control. I’m not using React here (because I’m not comfortable enough with it yet) but I’m practicing the “single source of truth” by putting everything that deals with the logic of the program in the script side.

If anybody is interested in seeing the complete code, here’s the link. There’s always a room for improvement, but my code passed all tests, so I’m happy :slightly_smiling_face: with the way it is.
https://codepen.io/thomasotani/pen/YzNBVEY

2 Likes

This is fantastic! A State Machine is new to me and this is a great example. However, I think I noticed one small issue when starting a new calculation. If you do something like 1 + 1 then press ‘equals’ the display changes to 2. If you then press 3 the display changes to 3 and then if you press ‘+’ then 2 the display changes to 32.

Overall, your calculator is great. Thanks for sharing really appreciate it, and congrats on passing all tests :slightly_smiling_face:

Yes, indeed. I reproduced the error. I’m getting this error

TypeError: calculator.op is not a function. (In 'calculator.op(calculator.leftOp, calculator.rightOp)', 'calculator.op' is undefined)

Interesting. I’m getting this error only on CodePen. I’m not getting any error when running locally using VS Code.
I’ll post when I find out why this discrepancy is happening.

Follow up post.

I posted the same code to JSFiddle and tested it. The code works fine :smiley: for the case you mentioned. I entered
1 + 1 = 3 + 2
the display showed 2 correctly (the last digit entered). Pressing = once more, the display will change to 5.
Here’s the link to JSFidde

https://jsfiddle.net/twotani/z30jpdso/2/

So I suspect that there’s some configuration that I’m not setting it right on CodePen. I’m curious why, so eventually I think I will try to search :face_with_monocle: for a correction on CodePen, but it’s a low priority item for me now. Basically it’s their problem, not mine. :wink:

1 Like

Ok, I am happy you figured it out because I checked locally and on Netlify and I get an error…Anyway good job and thanks for keeping me posted :slightly_smiling_face:

Okay, at least the error is consistent. I suspect it has to do something with JS being dynamically typed and different level of type checking is applied. CodePen etc are being stricter while others are not.

Since multiple places are detecting type error, I will try to find the cause.

I found the error. :sunglasses:

I need to apologize to CodePen first for hinting there’s some kind of configuration problem. It is purely my error. And all of my suspicion about JS is completely off the mark. My Javascript code was working fine all alone. It has nothing to do with Javascript being dynamically typed or anything. Sorry JS. Sorry CodePen.

The problem was the HTML file!!! :scream: Of course, it’s working fine on my local machine because I’m using the right HTML file. Of course, it works on JSFiddle because I uploaded all three (correct) files HTML, CSS, and JS. Apparently I forgot to upload a modified HTML file to CodePen. Since the code passed all the tests, I simply assumed all is good.

I was troubleshooting by barking up the wrong tree, focusing solely on the javascript code. I was finally able to detect the source of problem by tracing the state transition moves. When the EQ sign is entered, the state should become State.EQ (value 4), but somehow the state goes into State.OP (value 2). So instead of going from RT to EQ, pressing = goes from RT to OP. Because it was in the OP state, seeing 4 + is treated like (3 + 2 - 4 +). It tries to carry out 5 - 4. But in reality, we’re seeing 3 + 2 = 4 +. calculator.op is NONE at this point, so the type error (NONE “” is not a function).

Okay, drum roll :drum: please…
Here’s the correct element for the = sign in HTML

<button type="button" class="equal" value="=" id="equals" onclick="enterEq()">=</button>

If you look at the (old) wrong version, it is declared as

<button type="button" class="equal" value="=" id="equals" onclick="enterOp('=')">=</button>

It’s calling enterOp() instead of enterEq(). :confounded:
I made the correction, and it’s working correctly.

2 Likes

:clap: :clap: :clap: :clap: :clap: :clap: Way to go @twotani !!! Congratulations on finding the issue :+1:…Don’t worry I’m sure JS and CodePen forgive you :slight_smile:

Thanks for your detailed calculator example and keeping me updated on the issue. I’ve experienced similar issues when mixing HTML and JS. I wish you luck on the rest of your coding journey!

This topic was automatically closed 182 days after the last reply. New replies are no longer allowed.