updated calc logic #17
Conversation
js update
tal braun-js update
| @@ -0,0 +1,74 @@ | |||
| const numbers = document.querySelectorAll(".number"); | |||
There was a problem hiding this comment.
Good practice here to get all of the elements into 1 variable
| }); | ||
| }); | ||
|
|
||
| const addButton = document.getElementById("add"); |
There was a problem hiding this comment.
You should place all of your variables on top of your file
| addButton.addEventListener("click", () => { | ||
| display.textContent += "+"; | ||
| }); | ||
| subtractButton.addEventListener("click", () => { |
There was a problem hiding this comment.
Can you think of a way to add event listener too all of your operators? just like you did above
| display.textContent += "-"; | ||
| }); | ||
| multiplyButton.addEventListener("click", () => { | ||
| display.textContent += "*"; |
There was a problem hiding this comment.
Save your strings into constant variables
| equalsButton.addEventListener("click", () => { | ||
|
|
||
| const expression = display.textContent; | ||
| const parts = expression.split(/([+\-*/])/); |
There was a problem hiding this comment.
Save the regex inside a variable
|
|
||
| if(part == "*") { | ||
| const prevNumber = parseFloat(parts[i - 1]); | ||
| const nextNumber = parseFloat(parts[i + 1]); |
There was a problem hiding this comment.
You should check that this it not NAN
| const prevNumber = parseFloat(parts[i - 1]); | ||
| const nextNumber = parseFloat(parts[i + 1]); | ||
| const result = prevNumber * nextNumber; | ||
| parts.splice(i - 1, 3, result); |
There was a problem hiding this comment.
Fells like very specific logic, will it break if i add more parts? or add some complication to the assignment?
| parts.forEach((part, i) => { | ||
| if (part == "+") { | ||
| const prevNumber = parseFloat(parts[i - 1]); | ||
| const nextNumber = parseFloat(parts[i + 1]); |
There was a problem hiding this comment.
You can extract those 2 lines into a function to avoid repeating yourself
2 participants
No description provided.