Returning data from JavaScript object

Question

I would like to know what is the best way of doing the following. I have created an object which makes use of Ajax. It is dependent on the returned data to generate form tags etc. The data is a record which is turned into array for the value I need (NAME/STATUS).

var HTMLFormTag = (function() {

    var selectList;

    var generateSelectHTML = function(data, keySameAsValue, addselectvalue, selectedValue){
        selectList = document.createElement("select");
        selectList.setAttribute("class", "form-control");

        if(addselectvalue){
            var option = document.createElement("option");
            option.setAttribute("value", '');
            option.text = '--select--';
            selectList.appendChild(option);
        }
        $.each(data, function( index, value ) {
            var selected = '';
            var option = document.createElement("option");
            if(selectedValue.length > 0){
                if(value == selectedValue){
                    selected = 'selected';
                    option.setAttribute("selected", selected);
                }else{
                    selected = '';
                }
            }

            if(keySameAsValue){
                option.setAttribute("value", value);
            }else{
                option.setAttribute("value", index);
            }
            option.text = value;
            selectList.appendChild(option);
        });
    }

    return {
        createSelect : function(data,keySameAsValue, addselectvalue, selectedValue){
            generateSelectHTML(data,keySameAsValue, addselectvalue, selectedValue);
            return selectList;
        }
    }
})();


var CommsMatrix = (function() {

    // declare private variables and/or functions
    var statuses = [];
    var loadData = [];
    var protocols = [];

    var generateCMDStatuses = function() {
        loadData.push($.get( "/api/user/list/comms/matrix/data/format/json", {csrf: $.csrfParam('csrf'), select: 'distinct(status)' ,orderby: 'status'}).done(function(data){
            if (data.header.error) {
            } else {
                $.each(data.body.recordset.record, function( index, value ) {
                    if(value.STATUS != "to implement"){
                        statuses.push(value.STATUS);
                    }
                });

            }
        }));
    }

    var generateProtocolList = function() {
        loadData.push($.get( "/api/user/list/sys/list/protocols/format/json", {csrf: $.csrfParam('csrf'), orderby: 'name'}).done(function(data){
            if (data.header.error) {
            } else {
                $.each(data.body.recordset.record, function( index, value ) {
                    protocols.push(value.NAME);
                });

            }
        }));
    }

    return {
        createCM: function(params) {
              createCommsMatrix(params);
            },
        genCMDStatuses: function() {
            generateCMDStatuses();
        },
        genProtocolList: function() {
            generateProtocolList();
        },
        getCMDStatuses: function() {
            return statuses;
        },
        getSupportedProtocols: function() {
            return protocols;
        },
        getLoadData : function(){
            return loadData;
        }
    }

})();

In my main file to get the data I need:

$(function(){
    CommsMatrix.genCMDStatuses();
    CommsMatrix.genProtocolList();
    var loadData = CommsMatrix.getLoadData();
    var selectField = '';
    var selectProtocolField = '';

    t = $('table').DataTable( {
        "paging":   false,
        "ordering": false,
        "info":     false,
        "searching": false,
        }
    } );

    counter = 0;
    $('#addRow').on( 'click', function () {

        var inputSrcField = '<input size="15" name="src['+counter+']" class="form-control"/>';
        var inputDstField = '<input size="15" name="dst['+counter+']" class="form-control"/>';
        var nattype = ['source','destination'];
        var selectNatField = HTMLFormTag.createSelect(nattype, true, true, '');
        selectNatField.setAttribute("name", 'nat['+counter+']');
        selectNatField = selectNatField.outerHTML;

        var inputNatField = '<input size="15" name="nat['+counter+']" class="form-control"/>';

        var inputRemarksField = '<input name="remarks['+counter+']" class="form-control"/>';
        var textareaSrcPort = '<textarea name="srcport['+counter+']">any</textarea>';
        var textareaDstPort = '<textarea name="dstport['+counter+']">any</textarea>';

        t.row.add( [
            selectField,
            inputSrcField,
            '',
            textareaSrcPort,
            inputDstField,
            '',
            textareaDstPort,
            selectNatField,
            inputNatField,
            selectProtocolField,
            inputRemarksField,
        ] ).draw( false );

        counter++;

    } );


    $.when(loadData[0],loadData[1]).done(function(data){
        var statuses = CommsMatrix.getCMDStatuses();
        selectField = HTMLFormTag.createSelect(statuses, true, false, 'To implement');
        selectField.setAttribute("name", 'status['+counter+']');
        selectField = selectField.outerHTML;

        var protocols = CommsMatrix.getSupportedProtocols();
        selectProtocolField = HTMLFormTag.createSelect(protocols, true, false, 'TCP');
        selectProtocolField.setAttribute("name", 'protocol['+counter+']');
        selectProtocolField = selectProtocolField.outerHTML;

        $('#addRow').click();
    });
});

Is this an efficient way to do this?

Answer 1

$.get( "/api/user/list/comms/matrix/data/format/json", {csrf: $.csrfParam('csrf'), select: 'distinct(status)' ,orderby: 'status'}).done(function(data){

Use the more standard promise interface that's available for jQuery AJAX functions. Instead of done/fail/success/error, use then(successCb, failCb). Promises allow better flow control for async operations as well.

Even better, you can use the native fetch which is inspired by jQuery's ajax. It also returns a promise, albeit differently. It is always a success, you get the entire response (headers, body, etc.) and you decide what to do with it.

Speaking of promises...

$.when(loadData[0],loadData[1])

Native JS also has Promise.all which is also inspired by jQuery deferreds. It's like $.when except it accepts an array of promises instead of an args list of deferred objects. The resolved value of the promise returned by Promise.all is also an array of values resolved from the promises, in the order they were provided in the input array.

$(function(){
CommsMatrix.genCMDStatuses();
CommsMatrix.genProtocolList();

Instead of storing their deferred objects in an loadData, why not return their deferred objects, then feed them to $.when? Is there really a need to put it inside loadData? In fact, statuses and protocols are also the same thing.

Unless it's a cache of some sort, this intermediary array isn't necessary. Also, I'd leverage HTTP cache if you are doing caching. It's transparent to the code, so you don't do anything special (except for maybe ensuring headers are correct for caching and your request isn't busting the cache).

return {
    createCM: function(params) {
          createCommsMatrix(params);
        },
    genCMDStatuses: function() {
        generateCMDStatuses();
    },
    genProtocolList: function() {
        generateProtocolList();
    },
    getCMDStatuses: function() {
        return statuses;
    },
    getSupportedProtocols: function() {
        return protocols;
    },
    getLoadData : function(){
        return loadData;
    }
}

Whenever I see code like this, I cringe. Often times, this usually just translates to an object of functions. Skip the return {...} and the iife, and just use a straight-up object of functions instead.

    var inputSrcField = '<input size="15" name="src['+counter+']" class="form-control"/>';
    var inputDstField = '<input size="15" name="dst['+counter+']" class="form-control"/>';
    var nattype = ['source','destination'];
    var selectNatField = HTMLFormTag.createSelect(nattype, true, true, '');
    selectNatField.setAttribute("name", 'nat['+counter+']');
    selectNatField = selectNatField.outerHTML;

    var inputNatField = '<input size="15" name="nat['+counter+']" class="form-control"/>';

    var inputRemarksField = '<input name="remarks['+counter+']" class="form-control"/>';
    var textareaSrcPort = '<textarea name="srcport['+counter+']">any</textarea>';
    var textareaDstPort = '<textarea name="dstport['+counter+']">any</textarea>';

You might want to consider using a template library for this, like Mustache.

So for instance, your query code could be simplified into:

const CommsMatrix = {
  getCMDStatuses: function(){
    return fetch('/api/user/list/comms/matrix/data/format/json', {
      body: URLSearchParams({ csrf: $.csrfParam('csrf'), select: 'distinct(status)', orderby: 'status' })
    })
    .then(response => {
      return response.headers.error ? Promise.reject(response.headers.error) : response.json();
    }).then(body => {
      return body.recordset.record.map(v => v.STATUS)
    })
  },
  getSupportedProtocols: function(){
    return fetch('/api/user/list/sys/list/protocols/format/json', {
      body: new URLSearchParams({ csrf: $.csrfParam('csrf'), orderby: 'name' })
    })
    .then(response => {
      return response.headers.error ? Promise.reject(response.headers.error) : response.json();
    }).then(body => {
      return body.recordset.record.map(v => v.NAME)
    })
  }
}

Promise.all([CommsMatrix.getCMDStatuses(), CommsMatrix.getSupportedProtocols()])
  // Destructuring the 2-item array into statuses and protocols variables
  .then(([ statuses, protocols ]) => {
    // You've got statuses and protocols loaded
  })