How can I make my filter search code more efficient?

Question

Here is App.js

const listItems=document.querySelector(".student-list").children;
const studentList=document.querySelector('.student-list');
const numberOfItems=10;
const page=document.querySelector(".page");

displayPage=(list,pageNumber)=> {
   const SI=(pageNumber*numberOfItems)-numberOfItems;
   const EI=pageNumber*numberOfItems;
   Array.from(list).forEach((item,index)=> {
      if (index>= SI && index<EI) {
         item.style.display="block";
      } else {
         item.style.display="none";
      }
   })
}

addPaginationLinks=(list)=> {
   const pages=Math.floor(list.length/10)
   let html=``;
   for (let i=0; i<pages; i++) {
      if (i===0) {
         html+=`
         <li>
            <a class="active" href="#">${i+1}</a>
         </li>`;
      } else {
         html+=`
         <li>
            <a href="#">${i+1}</a>
         </li>`;
      }

   }
   const ul=document.createElement("ul");
   ul.innerHTML=html;
   const div=document.createElement("div");
   div.classList.add("pagination");
   div.appendChild(ul);
   page.appendChild(div);
}






displayPage(listItems,1);
addPaginationLinks(listItems);



addEventListener=()=> {
   const a=document.querySelectorAll("a");
   a.forEach(item=> {
      item.addEventListener('click', (e)=> {
         a.forEach(item=> {
            if (item.classList.contains("active")) {
               item.classList.remove("active")
               e.target.classList.add("active");
            }
         })
         const pageNumber=parseInt(e.target.textContent);
         displayPage(listItems,pageNumber);
      })
   })
}


addEventListener();


addSearchComponent=()=> {
   const pageHeader=document.querySelector(".page-header.cf")
   let html=`
   <div class="student-search">
      <input placeholder="Search for students...">
      <button>Search</button>
   </div>`;
   pageHeader.insertAdjacentHTML('beforeend', html);
}



addSearchComponent()

const search=document.querySelector("input");
const studentDetails=document.getElementsByClassName("student-details");



noResultsItem=()=> {
   const item=`
   <li class="no-results" style="display: none;">
      <h3>No Results Shown Found</h3>
   </li>`;
   studentList.insertAdjacentHTML('beforeend', item);
}

noResultsItem()


search.addEventListener('keyup', (e)=> {
   const noResults=document.querySelector(".no-results");
   const array=Array.from(studentDetails).filter(student=>student.children[1].textContent.includes(e.target.value))
   if (array.length==0) {
      Array.from(studentDetails).forEach(student=>{
         student.parentNode.style.display="none";
      });
      noResults.style.display="block"
   } else if (array.length>0) {
      noResults.style.display="none"
      Array.from(studentDetails).forEach(student=>{
         if (student.children[1].textContent.includes(e.target.value)) {
            student.parentNode.style.display="block";
         } else {
            student.parentNode.style.display="none";
         }
      });
   }
});   

I'm trying to refactor the Javascript code at the bottom of my app.js.

There's a keyup event in the search input, and when no results are found, I want to display the no results li item and also make it disappear immediately in case a user types in a character that creates a search match. However, I feel like I could have written this much better. I made the li's display none by default, but it feels tedious to re-alter it to display: none and display: block in both the if conditions.

Does anyone have any suggestions to a beginner programmer on how to make the code more efficient?

Answer 1

1) Avoid unnecessary document.querySelector and document.querySelectorAll. Store its results if you sure there won't be any changes.

Element searching in the DOM is relatively hard operation (it is extremely optimized, but still). So you should avoid it if you can.

You've already done a lot for that, but I still can see unsaved document.querySelector(".page-header.cf")

2) Use event delegation pattern.

You don't have to add event listeners to each element on the page. Especially if they are in the same container.

const parent = document.querySelector('#parent');

parent.addEventListener('click', (e) => {
  const target = e.target.closest('a');
  
  if (!target) return;
  
  if (target.matches('.active')) {
    target.classList.remove('active');
    
    resetPage();
  } else {
    target.classList.add('active');
    
    const page = target.textContent;
    
    displayPage(page);
  }
});

<div id="parent">
  <a class="page-number">1</a>
  <a class="page-number">1</a>
</div>

As a bonus, you don't have to add event listeners on newly created links or remove it from removed links.

3) Do not modify elements content as HTML string if you can avoid it.

In your case you can move li.no-results outside of studentList and show/hide it with display: none/block property. Or at least remove/append this element as a node, not as HTML-string. It is much faster.

4) Use debounce.

You don't need to trigger the search on each keyup. You need to trigger it only when user stops typing. So when you have event series with, let's say, 200 ms interval, you need to trigger search only for the last one. You can use lodash/debounce for that or write your own implementation

5) Learn how to use Chrome Profiling Tool It helps to find bottlenecks of your app and fix them.