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 a “onchange” Javascript event to check when the checkbox has been check, 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 in to the function. Then my “changeEmploy” function then 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.

Advertisements

About Alvin Bunk
Hi, I'm a software developer at Taft College.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s

%d bloggers like this: