Reducing code redundancy in a series of javascript function expressions

Question

I have a long series of javascript functions that are invoked by 'onClick' on the function name in the html.

    Corporate number <input type="text" autofocus id="corp_text">
    <button onclick="getCorpNum(this.form)">SAVE</button><br><br>

    LLC number <input type="text" autofocus id="LLC_text">
    <button onclick="getLLCNum(this.form)">SAVE</button><br><br>

    Type of nonprofit <input type="text" autofocus id="NP_text">
    <button onclick="getNonProfit(this.form)">SAVE</button><br><br>

    Other description <input type="text" autofocus id="OD_text">
    <button onclick="getDesc(this.form)">SAVE</button><br><br>

In the javascript the functions are very similar in form:

    /**
     * Get corporation number text field data.
     */
     var getCorpNum = function() {
         var Corp_number = [];
         Corp_number=document.getElementById("corp_text").value;
         if (Corp_number.length > 0) createJSONobj("Corp", Corp_number);
     }
    /**
     * Get LLC number text field data.
     */
    var getLLCNum = function() {
       var LLC_number = [];
       LLC_number = document.getElementById("LLC_text").value;
       if (LLC_number.length > 0) createJSONobj("LLC", LLC_number);
    }

    /**
     * Get type of non-profit text field.
     */
    var getNonProfit=function() {
        var NP_number = [];
        NP_number = document.getElementById("NP_text").value;
        if (NP_number.length > 0) createJSONobj("NP", NP_number);
    }

   /**
    * Get other description text
    */
    var getDesc=function() {
        var getDesc=[];
        getDesc = document.getElementById("OD_text").value;
        if (getDesc.length > 0) createJSONobj("Other_desc", getDesc);
    }

The issue is that I have other functions like these that all have the same form and I'd would like to reduce the amount of code. Something generic that would capture the form of the code. This is difficult for me because the onClicks need the name of of each function object.

Use unobtrusive javascript, create a plain object that maps a name or data attribute on your HTML to the uncommon elements in each function. — jdphenix, Apr 26 '15 at 01:45
You're passing in `this.form` but not using it. Is that intentional? — tandrewnichols, Apr 26 '15 at 01:48
Not really related, but from UX viewpoint.. that's a lot of button clicks for doing fine grain stuffs. Unless there is a very good reason, why make the user clicks tons of button when your app can do it from one place? Shouldn't there be only 1 save / submit button on the form or something and perform the extraction and json conversion there? — Jimmy Chandra, Apr 26 '15 at 02:00
Still not related... also, lots of autofocus... which one to autofocus? You can only have 1 focus, cmiiw. — Jimmy Chandra, Apr 26 '15 at 02:03

score 1 · Answer 1 · answered Apr 26 '15 at 01:51

1

You could just pass in the different parts.

Corporate number <input type="text" autofocus id="corp_text">
<button onclick="getThing('corp_text', 'Corp')">SAVE</button><br><br>

and then use those variables in a more generic function:

var getThing = function(id, name) {
     var values=document.getElementById(id).value;
     if (values.length > 0) createJSONobj(name, values);
}

But . . . maybe something more descriptive than "getThing" :)

answered Apr 26 '15 at 01:51

tandrewnichols

3,456
1
28
33

Thanks! That looks like what I need. Clearly I'm fairly new to javascript. Still learning it! – user2970900 Apr 26 '15 at 02:11

score 0 · Answer 2 · answered Apr 26 '15 at 01:49

How about this?

First, have each button call a function and pass in the type of form it is as a string argument

Corporate number <input type="text" autofocus id="corp_text">
<button onclick="getDetail('corp')">SAVE</button><br><br>

Then, have a flexible function that'll get the detail that you need:

 var getDetail = function(type) {
  var getElem = document.getElementById(type + '_text').value;
  if ((getElem).length > 0) {
    createJSONobj(type, getElem);
  }
 }

score 0 · Answer 3 · edited May 23 '17 at 11:51

Here's how I would approach this - you can refactor your function down to one by storing the different parts of the function calls outside. Note that I don't know what createJSONobj does in your code, so I created a dummy implementation so you can see console output.

Another notable change is not using the onxxx attributes in the HTML markup. Take a look over unobtrusive JavaScript and you'll see why.

function createJSONobj(k, v) {
  console.log('Dummy function ' + k + ', ' + v);
};

var jsonNameMap = {
  'corp_text': {
    k: 'Corp'
  },
  'LLC_text': {
    k: 'LLC'
  },
  'NP_text': {
    k: 'NP'
  },
  'OD_text': {
    k: 'Other_desc'
  }
};

document.getElementById('myForm').addEventListener('click', function(e) {
  if (e.target.nodeName === 'BUTTON') {
    var field = e.target.previousSibling.nodeType !== 3 ?
      e.target.previousSibling :
      e.target.previousSibling.previousSibling;

    if (field.value.length > 0) {
      createJSONobj(jsonNameMap[field.id].k, field.value);
    }
  }
});

<form id="myForm">
  Corporate number
  <input type="text" autofocus id="corp_text">
  <button>SAVE</button>
  <br>
  <br>LLC number
  <input type="text" id="LLC_text">
  <button>SAVE</button>
  <br>
  <br>Type of nonprofit
  <input type="text" id="NP_text">
  <button>SAVE</button>
  <br>
  <br>Other description
  <input type="text" id="OD_text">
  <button>SAVE</button>
  <br>
  <br>
</form>

Another thing of note: consider just have one save button. I'm not sure what your use case is but the "button for each field" approach is definitely a little busy.

Jimmy Chandra · Answer 4 · 2015-04-26T02:42:11.417

Just adding to the already obvious other answers... You can also choose to use data- attribute to hold values that you wish to pass to the calling function and combining it with convention like the <button> will always be after / preceeded by the <input> in such manner that when you click the button, you can use JavaScript to navigate back to its previous sibling (which is the <input>) and extract the needed values from it.

See the sample snippet... Open up your JavaScript console to see the actual outputs. The snippet also demonstrates how to consolidate the saving to one Save All button.

function createJSONobj(key, value) {
  var o = {};
  o[key] = value;
  var json = JSON.stringify(o);
  console.log(json);
}

function saveValue(el) {
  var input = el.previousElementSibling;  //get the input element next to the button
  saveJson(input);
}

function saveJson(input) {
  var key = input.dataset.jsonKey;        //get that input's data-json-key value
  var value = input.value;                //get the input's value
  
  if (value.length > 0) {
    createJSONobj(key, value);
  } else {
    console.log('Nothing to create.');
  }

}

function saveAll(frm) {
  //console.log(frm);
  
  var inputs = document.querySelectorAll('input', frm);  //get me all the inputs from the form
  //console.log(inputs);
  
  for(var i = 0, il = inputs.length; i < il; i++) {    
    var currentEl = inputs[i];
    saveJson(currentEl);
  }
  
}

<form id="theForm">
    
    Corporate number <input type="text" autofocus id="corp_text" data-json-key="Corp">  
    <button type="button" onclick="saveValue(this)">SAVE</button><br><br>

    LLC number <input type="text" id="LLC_text" data-json-key="LLC">
    <button type="button" onclick="saveValue(this)">SAVE</button><br><br>

    Type of nonprofit <input type="text" id="NP_text" data-json-key="NP">
    <button type="button" onclick="saveValue(this)">SAVE</button><br><br>

    Other description <input type="text" id="OD_text" data-json-key="Other_desc">
    <button type="button" onclick="saveValue(this)">SAVE</button><br><br>
    
    <!--better yet, instead of all those buttons -->
    <hr>
    <button type="button" onclick="saveAll(this.form)">SAVE ALL</button>
      
</form>

As for my own preference, I rather use binding library library like knockout or framework like angular to do this type of thing. They allow better separation of concern between what's UI stuffs versus application logic.

Reducing code redundancy in a series of javascript function expressions

4 Answers4