Click here to Skip to main content
16,015,809 members
Articles / Programming Languages / Javascript

JavaScript Code Refactoring

Rate me:
Please Sign up or sign in to vote.
4.75/5 (3 votes)
11 Apr 2017CPOL1 min read 7.2K   3   4
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.

JavaScript
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:

JavaScript
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:

JavaScript
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:

JavaScript
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.

License

This article, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)


Written By
Software Developer Taft College
United States United States
I’m a software developer. Currently I’m working at Taft College as a Programmer.

Comments and Discussions

 
QuestionAnother Way with JS Pin
rexbloom13-Apr-17 10:16
professionalrexbloom13-Apr-17 10:16 
AnswerRe: Another Way with JS Pin
Alvin Bunk14-Apr-17 8:45
professionalAlvin Bunk14-Apr-17 8:45 
QuestionNullReference Pin
thund3rstruck12-Apr-17 10:55
thund3rstruck12-Apr-17 10:55 
AnswerRe: NullReference Pin
Alvin Bunk14-Apr-17 8:44
professionalAlvin Bunk14-Apr-17 8:44 
The functions that call the function are all hard coded. They are dynamic.

But you do have a point that if I had dynamic code, yes you should check for null values. The point is, always keep your code simple. If your case was where you were doing something like that, then yes you need to add a whole bunch of various checks.

General General    News News    Suggestion Suggestion    Question Question    Bug Bug    Answer Answer    Joke Joke    Praise Praise    Rant Rant    Admin Admin   

Use Ctrl+Left/Right to switch messages, Ctrl+Up/Down to switch threads, Ctrl+Shift+Left/Right to switch pages.