Array.push() not working in React; I'm know you can't change state through push()

lasjorg, thank you so much, I really really appreciate you taking the time to look at this.
Test 11 should be passing now.
I need to look into edge cases more, especially having multiple zeros before a decimal, I hadn’t even considered that.
I will definitely add keyboard support when I have a bit more time.
I’ve added some hover: CSS for the buttons as well now.
I’m so glad to hear you say that I’ve picked up the React way of doing things.
It’s also very interesting to hear you say that this project is mainly Javascript logic and nothing to do with React (my understanding was that React is in essence just Javascript logic; I’ve clearly got a lot more to learn about React! I will start to delve into the course you linked me a couple of days ago)
Again, thank you so much!

I really don’t mean to offend, if you wouldn’t mind enlightening me as to what I am doing to seem not worthy of serious attention, I would really like to improve on that; thank you!

Good job passing all the tests.

Just to be clear, React is just JavaScript. But there is a difference between doing state/view management, making components, having them “talk” to each other, passing state around, mapping over data and returning JSX, etc. “React stuff” and doing plain JavaScript logic, like evaluating user input and making decisions or transforming input as needed. The plain JS I’m talking about is all the logic of input handling and making the correct decisions based on the data. Which is most of the apps core code if you really think about it.

lasjorg said he didn’t have a chance to look at your code - I’ll do that.

First off - global variables? Uuuuggghhh! Those should be in the ancestor component that is common to all that need it and passed up and down. This gets awkward in big apps - a problem redux will solve. Of course here, you only have one component.

Second, you only have one component. While you can kind of get away with that with such a trivial app, you should be developing good habits. Things should be broken down. I would would have structured it something like:

Calulator -
          |
          | -- Display
          |
          | -- Controls -
                        |
                        | - Button (x17)

There are probably variations - for example, you could group different buttons. But this is the basic idea.

Third, learn when to use const. For example:

    let calcArr = this.state.matharr.flat(i);
    let calcString = calcArr.join("");

The first one should be const because it is a reference type and you aren’t going to change the reference (in this case you’re not even changing what it refers to). The second one should be a const because you aren’t going to change it.

This line struck me as odd:

      this.setState({matharr:[this.state.matharr,num], currentNum:[num]})

Are you sure you didn’t mean:

      this.setState({matharr:[...this.state.matharr,num], currentNum:[num]})

Maybe that’s why you keep having to use .flat, you keep nesting your arrays.

Things like: this.state.currentNum=="/" - always use === unless you really want to allow coercion.

Things like if(ans!==undefined){ always strike me as odd. Usually you just do if (ans){ checking if it's a number. But of course you might have 0there so I guess it kind of makes sense. Still I always find it weird to define something as undefined - I usually usenull` instead. But there are differing thoughts on that.

Work on your formatting. It’s a good habit to get into. I know it seems trivial, but clean well formatted code is sooooo much easier to work with. Things like consistent indenting, etc. When you get to the point of writing things locally on an IDE (not codepen) install a linter to get a consistent way of doing things.

Just off the top of my head, if you put this in https://validatejavascript.com/, you can see a lot of issues. Note, some of them aren’t your fault - like saying React is undefined - that’s because codepen is handling it behind the scenes.

But that mainly deals with potential problems. There’s also the point of clear formatting. This is what you have for the decimal method:

  decimal(){
    
     if(!this.state.currentNum.flat(i).includes(".")  ){
    if(ans!==undefined){
      ans=undefined;
      this.setState({matharr:["0."],currentNum:["0."]});
      i=0
    } else if(this.state.matharr == ""){
      this.setState({matharr:["0."],currentNum:["0."]});
      
    } else {
    i++; 
      
      
      if(this.state.currentNum.flat(i).includes("0.")){}
      
    else {this.setState({matharr:[this.state.matharr,"."], currentNum:[this.state.currentNum,"."]}) }
      
      
    }
     
     
     }
  }

This is what I think it should look like:

  decimal() {
    if (!this.state.currentNum.flat(i).includes(".")) {
      if (ans !== undefined) {
        ans = undefined;
        this.setState({ matharr: ["0."], currentNum: ["0."] });
        i = 0;
      } else if (this.state.matharr == "") {
        this.setState({ matharr: ["0."], currentNum: ["0."] });
      } else {
        i++;
        if (this.state.currentNum.flat(i).includes("0.")) {
        } else {
          this.setState({
            matharr: [this.state.matharr, "."],
            currentNum: [this.state.currentNum, "."]
          });
        }
      }
    }
  };

I find that much easier to read and see the relationships. For example. I now notice that the last if statement has nothing in its code block - I didn’t notice that before. That code will be so much easier to read, especially by a stranger (which you will be after a few months away from the code.) Always clean as you go. Always imagine your mother is watching over your shoulder, telling you to put your dirty socks in the hamper. Don’t put it off or it won’t happen. Just get in the habit. These kinds of things a good linter will also catch. eslint with the airbnb rules is a common standard. For now, you can plug your code into something like prettier.io and see what it does.

Oh well, just some observations. Don’t let me discourage you, you’re on the right path, just keep it up.

1 Like

Ahhh okay, I understand now, thank you so much for the clarification!

Kevin, that’s incredible, I feel like I learnt a lot from your critique; a lot I had not even considered.

Global variables in React are bad (use ancestor components instead).

Applications should be broken down and placed in separate components.

I really need to get to grips with const and let; even though I know what it means, when constructing a variable, for some reason I really can’t tell at all whether it is going to be changed or not, so I always just use let.

Clean formatting makes a big difference, I will force my self to get into the habit of doing it properly

Wow, thats a lot of errors!

Thank you so much for links and the tips, I will use them all!

That’s fine, use let everywhere if you want – imo it signals that you are going to modify the value assigned variable, so personally I avoid it, but a lot of people seem to disagree.

But I think you’re right to get more comfortable with them: if you’re unable to use const almost everywhere, there may be an issue with your code (this is completely context-sensitive, so very much possibly, not definitely). ie if your reason for using let everywhere is just "I understand the trade-off but don’t think there’s any point in const" it will annoy a % of other people who need to read your code. But if it’s "I’m unsure whether the value assigned to any value in my program will change, so I avoid const", that possibly indicates underlying issues. It’s relatively uncommon to need to reassign any variables in React code.

1 Like

I would say that most people would say that almost all global variables are bad. Generally, you want to control the “scope” of variables and have that scope be as small as you can for each variable. There are a few exceptions, but not many.

1 Like