Skip to content

Conversation

stankovics
Copy link

Event delegation will be much better option instead of forEach() on numberButtons and operationButtons. With forEach() you are creating events for all buttons. That is a bad practice.
With event delegation you don't create click event for each button, just for buttons that are clicked.
This is my solution:

  1. And in HTML file for each number button and operation button data-set value of number for number buttons and operation sign for operation. data-number="1" & data-operation="" For example:
    1
    2
    ÷

  2. Select common parent element for all buttons:
    const parentElement = document.querySelector('.calculator-grid');

  3. Replace forEach on numberButtons and operationButtons with this code:

parentElement.addEventListener('click', function (e) {

if (e.target.closest('[data-number]')) {

const btnNum = e.target.closest('[data-number]');
const selectedNumber = btnNum.getAttribute('data-number');
calculator.appendNumber(selectedNumber);
calculator.updateDisplay();

}
if (e.target.closest('[data-operation]')) {
const btnOperation = e.target.closest('[data-operation]');
console.log(btnOperation);
const selectedOperation = btnOperation.getAttribute('data-operation');
console.log(selectedOperation);
calculator.chooseOperation(selectedOperation);
calculator.updateDisplay();
}
});

replaced button => {code}
with ()=>{code}

button argument in this case doesn't have any purpose.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant