Need help with my React calculator

Ok, so i have a function that is called when I click on the calculator button

handlePress(e){
    if(e.target.value !== "+" && e.target.value !== "-" && e.target.value !== "x" && e.target.value !== "/"){
     ...

    } else {
      this.pushValue(this.state.currentValue);
      this.updateScreen(e.target.value);
    }
  }

As you can see, when I press one of the operations buttons (+, -, x, /) it is supposed to run a function pushValue(), here it is:

pushValue(){
    this.setState({
      currentEquation: this.state.currentEquation.concat(this.state.currentValue),
    });
    this.setState({
      currentValue: "0",
    });
  }

It is supposed to 1) Push the current value to the equation array 2) Clear the current value by setting it to zero. The first part of that happens fine, but for whatever reason, it never resets the currentValue.
Initialy I had both of these state changes in the same setState, but i separated them thinking maybe this was the problem.
What do you think is the problem here?

Here is the link to the full app if the code above is not enough.

  1. Here you’re passing a value
 this.pushValue(this.state.currentValue);

but the definition is

pushValue(){
  1. When updating a state based on the the value of the previous state, you need to use that previous state, to ensure the update is carried out correctly. It should be something like
this.setState((prevState) => {
      currentEquation: prevState.currentEquation.concat(this.state.currentValue)
    });
  1. Define separate click handlers instead of one handler with tests inside. You can define handleDigitPress for handling digit button press and handleOperator for handling the pressing of operator buttons, for example.

Thanks for the tips, this is definetly some solid advice, but the issue still perisits.

Here is what my operation handler and pushValue look like now

handleOp(e){
    this.pushValue(this.state.currentValue);
    this.updateScreen(e.target.value);
  }

 pushValue(value){
    this.setState((prevState) => ({
      currentEquation: prevState.currentEquation.concat(value),
      currentValue: "0",
    }));
  }

But it stil completely ignores me setting currentValue in the second function.

Not sure what you’re trying to do here, but what if you ‘clear’ currentValue: with "", not "0"? or is this behavior on purpose?

Ok so this seems to have fixed it? I had some logic in there that depends on currentValue being zero (if it’s zero then the next number will not be added to the value, but will instead replace it so I will not end up with weird numbers like “0123”), but it turns out making pushValue reset it to an empty sring doesnt really change anything.

But i still don’t understand why setting it to zero was ignored in the pushValue function. My best guess is that because setState is asynchronous, the state update that sets currentValue to zero and the one setting it to the operation symbol (+, -, *, /) are batched together so instead of it being 0 then +, for example, it was 0+. I don’t know the way to check this is definetly the cause though.

I don’t like not understanding things :frowning:

Why do you still have a test inside handleNum?

 handleNum(e){
    if(this.state.currentValue != "+" && this.state.currentValue != "-" && 
    this.state.currentValue != "x" && this.state.currentValue!= "/"){

handleNum is called from digits only.

I think you want to rethink how to set up pushValue and updateScreen. It will be easier if you call pushValue only when the full operand or operator is available. For example, while entering digits like 4 5 1, just update the display (updateScreen). When it is done, like an operator or equal button is pressed, then push the value in the display to the equation.

Think that is something about your logic, not react.
See, @twotani reads your code the same way that i do ( e.g. condition in handleNum etc), we expect you to deal with the value of pressed button (e.target.value or something like this), but you test this.state.currentValue (value of the most recent pressed button(?)), then update state with it?
it’s difficult to follow.

My suggestion is get rid of additional functions (updateScreen, pushValue, try to make it as simple as possible (handlers for different types of buttons)) and functioning, maybe reconsider naming. Refactor later if you want to.

To see how it’s working a bit better debugger; can be useful too

Yep :face_with_head_bandage:

@twotani Well, this check is there so if the screen is currently displaying an operand symbol and a digit is pressed, it will push the value to hte equation, so i dont end up stuff like +1234 on the screen, it can either be an operand or a number, but it looks like I’ll need to rethink my logic, as you both suggested.

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