JavaScript Calculator - can I get a sympathy pass?

Salutations!

I made a JS Calculator and I’m pretty sure it satisfies all the user stories but it won’t pass the tests. I think it’s because I don’t allow the user’s expression to chain operators - instead it immediately evaluates the prior expression and applies the new operator to the result

It’s hard to explain so here are the links to my calculator and the github repo if you wanna see the code

My Calculator

GitHub Repo

So for example if a user presses 2 + 3 + 4

The sample project on codepen would add it all to the expression and not evaluate anything until ‘=’ is pressed

My calculator, when the second ‘+’ is pressed will evaluate the 2 + 3, and a new expression is started on the result, so the new expression would be ‘5+’. Then when the 4 is pressed the expression updates to ‘5+4’.

I think this immediate execution thing keeps the tests from passing even tho it functions according to the user stories

Was wondering if I could get a sympathy pass :sweat_smile:

Would love any feedback / bug reports, spent alot of time on this sucker

Many thank you’s!

2 Likes

Omg, it’s beautiful! What’s your twitter, youtube, instagram so I can follow you.

It should support both you can read the Note On Calculator Logic on the challenge page at the bottom.

You can see it in the test messages as well.

The expression 3 + 5 * 6 - 2 / 4 should produce 32.5 or 11.5

Some of the tests that are failing also do not seem related to calculations.

If I said that to my QA team at work, they’d send me my last paycheck wrapped in a flyer for the local barber college. :wink:

Part of the job is figuring out how to meet the requirements. You have to keep telling yourself - failure is not an option.

1 Like

I think the problem might be that I store the user’s input numbers as strings instead of as numbers?

The ‘clear’ button test fails (I think) because the test expects the calculator to display 0 and mine displays “0”?

But the user stories don’t say which data type you have to store or display the numbers as. Going through the tests my calculator is failing, when I perform the test manually it performs exactly the way the user story describes with immediate execution logic.

I’ll have to come back to it tho, I’ve hit pause on learning node for too long while working on this.

Thank you for checking it out!

I have always wished I was able to give myself (good) haircuts!

You are 100% right. When I started working on this I peeped the sample project’s regex hell and made my own highest-priority user story of not doing it that way. Probably shouldn’t prioritize avoiding something I could def use some practice with.

But I’m still convinced the tests have some specificities that aren’t defined in the user stories that are keeping my calculator from passing. But I’m also convinced I’m very tired and could be completely and embarrassingly wrong.

Random question tho, is the eval() method not dangerous in this application because I’m restricting the input characters to just numbers and a few symbols? I had nightmares last night of hackers crashing the github pages server because of my calculator, woke up in a cold sweat :scream:

Displaying, writing or printing is a symbolic representation. JS can work on numbers as a data type, but the data type used for visually representing numbers are strings, or characters really (which internally is just numbers, i.e. character encoding). The page is just a text document. It doesn’t contain numbers, only symbols.

You perform math on number types and display them using string types.

Random question tho, is the eval() method not dangerous in this application because I’m restricting the input characters to just numbers and a few symbols?

Yes, it is dangerous. In a real world app you would want to come up with a better solution. But that would make this project too difficult. In reality, I’m sure there are libraries out there that can handle this.

I had nightmares last night of hackers crashing the github pages server because of my calculator, woke up in a cold sweat

I’m no security expert, but I’m not sure how that would happen. I assume that the gitpages server is insulated from the code it is serving. At best they could wreak havoc on your little calculator app. I think they could disrupt that instance and slow down the server. But since it is just a static page, I don’t know what they could do any real damage in this case.

But I’m no expert.

There’s no risk to using eval() in this context purely on the client side as you’re not sending data to a server so code injection isn’t possible, the input values of a calculator wouldn’t allow for it anyway,

Avoid using eval for this though I used it on my calculator and the performance is rubbish

Figured out the issue… only 2 months later

I confirmed my calculator functioned perfectly, but I was mapping the buttons from an array of numbers and an array of operators.

I hardcoded all the buttons instead and it instantly passed.

My only guess is that mapping the buttons might cause unnecessary re-renders that are too slow for the test?

Is this bad practice to use arrays I define to map/create DOM elements in React?

I have another app where I map buttons too and it crashes on mobile only… wondering if it’s the same problem

My only guess is that mapping the buttons might cause unnecessary re-renders that are too slow for the test?

Is this bad practice to use arrays I define to map/create DOM elements in React?

No, that should be fine. It would depend on the implementation. I would guess that somehow you created a different DOM structure.

It was something with the test pressing the buttons faster than my app could handle

“Test expect ‘1’ to be ‘123’”

But manually entering 123 would work fine

I tried different approaches all with the mapping but hard coding the buttons was the only thing that worked

Sure. I just know that this has been solved with mapping the buttons. And that would be a typical way to solve this. It is a very common practice in React.

Ya that’s what I figured since we map data from API’s all the time

const numbersArray = [
    { id: 'zero', value: '0' },
    { id: 'one', value: '1' },
    { id: 'two', value: '2' },
    { id: 'three', value: '3' },
    { id: 'four', value: '4' },
    { id: 'five', value: '5' },
    { id: 'six', value: '6' },
    { id: 'seven', value: '7' },
    { id: 'eight', value: '8' },
    { id: 'nine', value: '9' }
]

const operatorsArray = [
    { id: 'divide', symbol: '/' },
    { id: 'multiply', symbol: 'x' },
    { id: 'subtract', symbol: '-' },
    { id: 'add', symbol: '+' }
]
  
  // ** MAPPING OUT THE NUMBER BUTTONS **
  const numberButtons = numbersArray.map((obj) => (
    <button
      key={nanoid()}
      id={obj.id}
      className="btn btn-number"
      type="button"
      onClick={() => handleNumber(obj.value)}
    >
      {obj.value}
    </button>
  ))

  // ** MAPPING OUT THE OPERATOR BUTTONS **
  const operatorButtons = operatorsArray.map((obj) => (
    <button
      key={nanoid()}
      id={obj.id}
      className="btn btn-operator"
      type="button"
      onClick={() => handleOperator(obj.symbol)}
    >
      {obj.symbol}
    </button>
  ))

This is how I mapped it, thought it was pretty basic. Any idea why this would screw up the tests?

Here’s the final codepen since the gh-pages one is old

Yeah, I’m not seeing why that wouldn’t work. I don’t like using random values for keys, but I don’t think that would cause the problem and if you are doing it outside a component (therefore not being rerun with different values) then it shouldn’t matter anyway - still, why not use the id?

But yeah, I wouldn’t worry about it too much.

1 Like

It’s the way you are using nanoid. When I tested the mapped buttons with a fixed key (like the id) it passed the tests. If you move the object arrays out of the component and create a key property using nanoid it passes as well.

2 Likes

You’re totally right, why am I using nanoid? Totally unnecessary, probably just blindly repeating a tutorial I was doing at the time, where it made sense.

It’s kind of awesome that made the tests fail cause it is totally unnecessary. Not sure why I moved the variables into the component too

Thank you guys for your help, really appreciate it!

1 Like

Well, there are cases where the mapped data doesn’t contain unique values so you need to generate it, and using an uuid is a good way to make sure it is unique. But in this case, as you are in control of the data and can add whatever you want using something simpler for the keys does make sense.

1 Like

I see a lot of people use some kind of random generator for keys. Just to reiterate, it is fine if it happens once, but React is counting on those keys to be the same each time. If you throw a random key on there, it’s fine as long as it doesn’t change. But if the data has something unique to each element already, itls better to just use that. Butlike I said, I’ve seen that mistake a lot, even in tutorials.

1 Like