Tip Calculator App

Community feedback

• 

  P

  Valeria Montoya•450

  @ValeriaMontoya

  Posted 3 months ago

  Hi @EmLopezDev 👋🏻

  Your JavaScript code is generally well-structured and understandable. However, there are a few areas where improvements can be made. Here’s a breakdown:

  Typo

  const addAcitveTip = (evt) => { ... }
  

  addAcitveTip should be addActiveTip.

  Repeated DOM manipulation
  You're calling tipButtons.forEach() multiple times in both removeActiveTip() and addAcitveTip(). These can be merged for better performance and clarity:

  const setActiveTip = (evt) => {
      tipButtons.forEach((button) => {
          const isActive = button.id === evt.target.id;
          button.classList.toggle("active", isActive);
          if (isActive) tipCustom.value = "";
      });
  };
  
  const addTip = (evt, value) => {
      setActiveTip(evt);
      tip = value ? Number(value) : null;
      showResults();
  };
  

  Inefficient showResults() logic
  You're checking the same condition twice with a lot of repeated code. Combine conditions using else and simplify:

  const showResults = () => {
      const errors = {
          bill: !bill,
          tip: tip === null,
          people: !people,
      };
  
      billErrorSpan.innerHTML = errors.bill ? "Can't be zero" : "";
      tipErrorSpan.innerHTML = errors.tip ? "Must select tip" : "";
      peopleErrorSpan.innerHTML = errors.people ? "Can't be zero" : "";
  
      if (errors.bill || errors.tip || errors.people) {
          tipPerPersonSpan.innerHTML = "$0.00";
          totalPerPersonSpan.innerHTML = "$0.00";
          resetButton.setAttribute("disabled", true);
      } else {
          const perPerson = formattedResults();
          tipPerPersonSpan.innerHTML = perPerson.tip;
          totalPerPersonSpan.innerHTML = perPerson.total;
          resetButton.removeAttribute("disabled");
      }
  };
  

  I really hope my feedback helped, keep coding! 👩🏻‍💻

  11

  Marked as helpful