JavaScript Code Refactoring

Introduction

Recently, I have been working on a multi-step (has Back and Next buttons) HTML form (in Symfony) where the various sections of the form have the same “Other” checkbox. I use an “onchange” JavaScript event to check when the checkbox has been checked, I then display the “Other” Label and input field. The problem I had was because there were multiple “Other” checkboxes, they all have different “id“s, and this is a problem because I needed to handle the different ids with one function.

Original Function

Here is the original function before code-refactoring showing how I get the elements and check if the Other checkbox is checked and then change the CSS styling.

function changeEmploy(){
   // Get elements.
   var employOther = document.getElementById( "ind_needs_assess_employ_9" );
   var otherLabel = document.getElementById( "employ_other_label" );
   var otherInput = document.getElementById( "ind_needs_assess_employ_other" );

   // Check if Other is selected.
   if (employOther.checked){
      otherLabel.style.display = "inline";
      otherInput.style.display = "inline";
   }
   else{
      otherLabel.style.display = "none";
      otherInput.style.display = "none";
   }
}

The if statement simply checks if the checkbox is checked, then I will set the CSS display value for the Other Label and input to either “inline” to show them, or “none” to make them hidden.

Code-Refactoring

Since I had a total of seven (7) of the “Other” checkboxes on my entire form, it didn’t make sense to have seven similar functions, that essentially do the same thing. So I decided to code-refactor and make one main function and pass in the id values for the elements in an Array.

The simplified code is like so:

function handleCheckbox( values ){
   // Get elements.
   var checkbox = document.getElementById( values[0] );
   var otherLabel = document.getElementById( values[1] );
   var otherInput = document.getElementById( values[2] );

   // Check if Other is selected.
   if (checkbox.checked){
      otherLabel.style.display = "inline";
      otherInput.style.display = "inline";
   }
   else{
      otherLabel.style.display = "none";
      otherInput.style.display = "none";
   }
}

In this case, the values parameter is an Array passed into the function. Then my “changeEmploy” function becomes:

function changeEmploy(){
   var values = new Array(
      "ind_needs_assess_employ_9",
      "employ_other_label",
      "ind_needs_assess_employ_other"
   );

   handleCheckbox( values );
}

This is much simpler, and I can re-use the “handleCheckbox” function with my function “changePersonal” function like so:

function changePersonal(){
   var values = new Array(
      "ind_needs_assess_personal_5",
      "pers_other_label",
      "ind_needs_assess_pers_other"
   );

   handleCheckbox( values );
}

So I simply pass in an Array with the element ids as a parameter to the “handleCheckbox” function.

End Result

This now simplifies my code, makes my code more readable, and also more supportable and maintainable. Hopefully this helps someone.