Design comparison
Community feedback
- @mofadaPosted 3 months ago
Your JavaScript code is well-organized and structured. However, there are some suggestions for improving readability, maintainability, and efficiency:
1. Optimization and Best Practices:
-
Debounce Input Events: Currently, the event listeners for input fields trigger calculations every time the user types something. To improve performance, especially with large inputs, consider debouncing the input event listeners.
function debounce(func, wait = 300) { let timeout; return function (...args) { clearTimeout(timeout); timeout = setTimeout(() => func.apply(this, args), wait); }; } inputBill.addEventListener("input", debounce(function () { values.bill = parseFloat(this.value); if (!isNaN(values.bill)) { calculate(); } }));
-
Consistent Variable Naming: The variable
alert
can be confusing since it's a global function in JavaScript. Consider renaming it to something more descriptive, likealertMessage
oralertElement
.const alertMessage = document.querySelector("#alert");
2. Maintainability:
-
Separate Calculation Logic: The
calculate
function is doing a lot of work. You could separate the logic for calculating the tip amount and the total amount into individual functions. This will make the code easier to test and maintain.function calculateTipAmount(bill, tip, people) { return (bill * tip) / 100 / people; } function calculateTotalAmount(bill, tip, people) { return ((bill * tip) / 100 + bill) / people; } function calculate() { const { bill, tip, people } = values; resultTipValue.innerText = `$${calculateTipAmount(bill, tip, people).toFixed(2)}`; resultTotalValue.innerText = `$${calculateTotalAmount(bill, tip, people).toFixed(2)}`; }
-
Reuse of CSS Classes: In the
showAlert
andcleanAlert
functions, you're adding and removing multiple classes for styling. Consider grouping these styles into a single CSS class, like.alert-active
, to reduce redundancy..alert-active { border: 2px solid #CA8073; }
Then, in your JavaScript:
function showAlert(reference) { alertMessage.textContent = "Can’t be zero"; reference.classList.add("alert-active"); } function cleanAlert(reference) { alertMessage.textContent = ""; reference.classList.remove("alert-active"); }
3. Error Handling:
-
Handle NaN in
calculate
: Ensure that thecalculate
function can handle cases where the input values might not be valid numbers (NaN). This can prevent unintended results being displayed.function calculate() { const { bill, tip, people } = values; if (isNaN(bill) || isNaN(tip) || isNaN(people) || people === 0) { resultTipValue.innerText = `$0.00`; resultTotalValue.innerText = `$0.00`; return; } resultTipValue.innerText = `$${calculateTipAmount(bill, tip, people).toFixed(2)}`; resultTotalValue.innerText = `$${calculateTotalAmount(bill, tip, people).toFixed(2)}`; }
4. Reset Function:
-
Complete the
reset
Function: You have an emptyreset
function. Consider moving the reset logic from thebtnReset
event listener to this function for better code organization.function reset() { values.bill = 0; values.tip = 5; values.people = 1; inputBill.value = 0; inputCustom.value = 0; inputPeople.value = 1; resultTipValue.textContent = "$0.00"; resultTotalValue.textContent = "$0.00"; removeActive(); } btnReset.addEventListener("click", reset);
5. General Suggestions:
-
Avoid Global Scope Pollution: Consider encapsulating your code within an IIFE (Immediately Invoked Function Expression) to avoid polluting the global scope.
(function() { // All your code here... })();
By applying these suggestions, your code will be more efficient, readable, and easier to maintain.
0 -
Please log in to post a comment
Log in with GitHubJoin our Discord community
Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!
Join our Discord