DEV Community

loading...
Cover image for Refactor to Reduce Repetitive Code

Refactor to Reduce Repetitive Code

maasak profile image Maasa Kono ・2 min read

One of the principles of software development I've been taught is to keep your code DRY. DRY stands for "Don't Repeat Yourself", and the idea is to minimize repetitive lines of code.

The Clunky Code

In my previous post, we added a feature to our bug tracking app to filter our list of issues by priority level. Here is what I coded to get this initially working:

document.getElementById('all').addEventListener('click', e => {
  renderCurrentData();
})

document.getElementById('low').addEventListener('click', e => {
  document.getElementById('issuesList').innerHTML = "";
  readFilteredIssues(e);
})

document.getElementById('medium').addEventListener('click', e => {
  document.getElementById('issuesList').innerHTML = "";
  readFilteredIssues(e);
})

document.getElementById('high').addEventListener('click', e => {
  document.getElementById('issuesList').innerHTML = "";
  readFilteredIssues(e);
})

function readFilteredIssues(e) {    

 issuesRef.orderByChild('priority').equalTo(e.target.innerHTML).on("value", function(snapshot) {
    snapshot.forEach(snap => {
      const issue = snap.val();

      document.getElementById("issuesList").innerHTML +=
      //  HTML code for each issue element
    })
  }
)}
Enter fullscreen mode Exit fullscreen mode

As you can see, this is extremely repetitive! Whenever you notice repeated lines of code, you'll know that it's time to do some refactoring, which is what we'll do here now.

The Cleaner Code

The first thing I notice here is that we are firing off the same function with the same click event. The only difference is the priority level (or filter option) that is being passed into the readFilteredIssues() function. We will want to query the div that holds all the buttons for filtering options, grab the value of the innerHTML, and pass that through to readFilteredIssues():

const filterButtons = document.querySelector('.filterButtons');

filterButtons.addEventListener('click', filterIssues);

function filterIssues(e) {
  document.getElementById('issuesList').innerHTML = "";
  const priorityLevel = e.target.innerHTML;
  readFilteredIssues(priorityLevel);
}
Enter fullscreen mode Exit fullscreen mode

That alone has already reduced our lines of code significantly! Now we just need to make minor changes to the readFilteredIssues() function.

First, instead of passing e.targetinnerHTML to the equalTo Firebase method, since we're already getting the innerHTML from the previous filterIssues() function, we just need to pass in the same argument being passed into the function itself (which can be named anything). For our purposes, I will rename it to filterOption, just because it would make a little more sense.

The other change we will make is to add a conditional statement for the filter option of "All", which isn't a priority level. This is just an option to show all issues in our database. Before doing anything else in this function, we will check to see if the parameter passed in is equal to "All", and if it is then we will just render all data (which is a function we already have). Otherwise, we will get the filtered data to render.

The code will look like this:

function readFilteredIssues(filterOption) { 
  if (filterOption === "All") renderCurrentData();

 issuesRef.orderByChild('priority').equalTo(filterOption).on("value", function(snapshot) {
    snapshot.forEach(snap => {
      const issue = snap.val();

      document.getElementById("issuesList").innerHTML +=
      //  HTML code for each issue element
    })
  }
)}

Enter fullscreen mode Exit fullscreen mode

And that's it! Our filtering function is working just as it was before, and we aren't repeating lines of code unnecessarily.

Discussion

pic
Editor guide
Collapse
zoedreams profile image