0

I am absolute begginer in jQuery. I wrote some code for my app and put it into .js file:

How should I avoid code repetition in jQuery? Where should I store my .js code (one huge .js file, couple of smaller or straight in html source)?

This is my .js file:

$(document).ready(function() {
    $("label[for='id_category1'], #id_category1, label[for='id_subcategory1'],#id_subcategory1 ").hide();
    $('select[name=subcategory]').empty();
    $('select[name=subcategory]').prepend('<option value="Not selected" selected disabled>Select Category...</option>');
    $('select[name=subcategory1]').empty();
    $('select[name=subcategory1]').prepend('<option value="Not selected" selected disabled>Select Category...</option>');
    // called when category field changes from initial value
    $('#id_group').change(function() {
        var $this = $(this);
        if ($this.find('option:selected').attr('value') == 'premium') {
            $("label[for='id_category1'], #id_category1").show();
            $("label[for='id_subcategory1'], #id_subcategory1").show();
        } else {
            $("label[for='id_category1'], #id_category1").hide();
            $("label[for='id_subcategory1'], #id_subcategory1").hide();
        }
    })


    $('#id_category').change(function() {
        var $this = $(this);
        if ($this.find('option:selected').index() !== 0) {
            category_id = $('select[name=category]').val();
            request_url = '/get_subcategory/' + category_id + '/';
            $.ajax({
                url: request_url,
                type: "GET",
                success: function(data) {
                    $('select[name=subcategory]').empty();
                    $.each(data, function(key, value) {
                        $('select[name=subcategory]').append('<option value="' + key + '">' + value + '</option>');
                    });
                }
            })
        }
    })
    $('#id_category1').change(function() {
        var $this = $(this);
        if ($this.find('option:selected').index() !== 0) {
            category_id = $('select[name=category1]').val();
            request_url = '/get_subcategory/' + category_id + '/';
            $.ajax({
                url: request_url,
                type: "GET",
                success: function(data) {
                    $('select[name=subcategory1]').empty();
                    $.each(data, function(key, value) {
                        $('select[name=subcategory1]').append('<option value="' + key + '">' + value + '</option>');
                    });
                }
            })
        }
    })
    $("label[for='id_keywords']").html("Keywords 0/100");
    $('#id_keywords').keyup(function() {
        var charsno = $(this).val().length;
        $("label[for='id_keywords']").html("Keywords (" + charsno + "/100)");
    });
    $("label[for='id_description']").html("Description 0/250");
    $('#id_description').keyup(function() {
        var charsno = $(this).val().length;
        $("label[for='id_description']").html("Description (" + charsno + "/2500)");
    });
});

Thank you for any clues to beginner.

jundymek
  • 1,013
  • 4
  • 18
  • 40
  • 1
    What you're asking is opinion based. Whether you should use one big file, or several small files, depends on the project and you, and has been [debated for years](http://stackoverflow.com/questions/15236767/one-big-javascript-file-or-multiple-smaller-files), and it's still a matter of opinion, even more so now that HTTP/2 makes multiple requests more efficient. What's certain though, is that you should generally store CSS in .css files, and javascript in .js files, and separate it from the HTML. As for code repetetion, smarter selectors and more variables do solve a lot. – adeneo Feb 05 '17 at 20:11
  • So adeneo you have stated what I have already answered with :) – Datadimension Feb 05 '17 at 20:19

4 Answers4

0

ALWAYS separate javascript from HTML.

ALWAYS separate and load separate javascript files according to separate functionality.

Then try and break down into MVC type architecture. I've been there where a javascript file is 2000 lines long - its unmanageable for editing and debugging.

ALWAYS try and use classes.

Class architecture is pseudo in Javascript but:

var view;
var model;
var ctrl;

/** - js_include_mvc.js
 * as per:
 * http://stackoverflow.com/questions/15192722/javascript-extending-class
 * provides the bare minimum facility to instantiate MVC objects if the base mvc classes are not subclassed
 * this ensures there is always js mvc
 *
 */
Ctrl.inherits(BaseCtrl);
function Ctrl(args) {
     BaseCtrl.apply(this, arguments);
     Ctrl.prototype.boot = function () {

     }
}

View.inherits(BaseView);
function View(args) {
     BaseView.apply(this, arguments);
     View.prototype.boot = function () {

     }
}

Model.inherits(BaseModel);
function Model(args) {
     BaseModel.apply(this, arguments);
     Model.prototype.boot = function () {

     }
}

Then for example:

function BaseView() {
    BaseView.prototype.somemethod = function () {
        //some functionality here
        return "I am a View";
    }
}

And finally call it in the global page script:

var view=new BaseView();
view.somemethod();

outputs:

"I am a view"

I term the class BaseView as I put code that is always reused here, however the BaseView class can be further extended according to application. So BaseView is used a a main repository for all my apps and I extend it according to requirements.

Datadimension
  • 872
  • 1
  • 12
  • 31
  • How can I separate functionality from my .js file? There are two parts of code. Top part is for operate choice fields in my form, bottom part is for displaying labels in the same form. Should I separate it? Is it possible to use two or more .js files in one page at all? For now I have one more jQuery script in my html file - it is for redirecting site after submitting form. – jundymek Feb 05 '17 at 20:20
0

Some of my code comments need a rewrite but my Ajax class goes like this:

if (SERVER_INTERFACES == undefined) {
  var SERVER_INTERFACES = {};//global server array required for named multiple servers and asynch data callbacks
}

/** Ajax_js - communicates with the server to retrieve data
 *
 * @param String newName is the name this class can reference for things like callbacks
 * @param Strin newUrl
 * @param String newNamedServer is the resource url defined as a php ajax server class
 * @returns {Ajax}
 */
function Ajax_m(newName, newUrl, newNamedServer) {
  this.namedServer = newNamedServer;
  this.interface_name = newName;
  this.url = newUrl;
  this.server_data; //allows a query to be given an id and the result data stored and accessible without having to manipulate into method arguments which might require data changes
  this.server_data = new Array(); //allows a query to be given an id and the result data stored and accessible without having to manipulate into method arguments which might require data changes
  this.request_index = 0;
  this.multiqueryindex = 0;
  this.multiqueryctrl = new Array();
  this.dataType = "json";
  this.reqType = "post";
  SERVER_INTERFACES[this.interface_name] = this;
  Ajax_m.prototype.request = function (requestobj, callback, optargs) {
    optargs = model.array_defaults(optargs, {runtagvals: {}});
    if (optargs.id == undefined) {
      this.request_index++;
    } else {
      this.request_index = optargs.id;
    }
    var request_id = this.request_index;
    if (typeof requestobj == "string") {
      //legacy was the requestobj as a string which is set to request and defaults assumed
      //attempt to parse ajaxRequestSubType as first 'word'
      var clauseend = requestobj.indexOf(" ");
      var ajaxRequestSubType = requestobj.substr(0, clauseend);
      requestobj = {ajaxRequestSubType: ajaxRequestSubType, request: requestobj, callback: callback, request_id: request_id};

    } else {
      //for legacy if callback and id are defined they are added but if in requestobj they will be overridden
      var args = {callback: callback, request_id: request_id};
      for (var r in requestobj) {
        args[r] = requestobj[r];
      }
      requestobj = args;
    }

    //ensure default settings
    var requestbuild = model.array_defaults(
            requestobj,
            {
              request: null,
              callback: "",
              errorCallback: null,
              request_id: null,
              interface_name: this.interface_name,
              responsedata: null,
              multirequestid: null,
              ajaxRequestSubType: "",
              ajaxRequestType: "database", //default to use database cfg definitions of site
              ismultiparent: false,
              _method: "PATCH"//for laravel5
            }
    );

    requestbuild.request = model.runtagreplace(requestbuild.request, optargs.runtagvals);

    this.server_data[request_id] = requestbuild;
    //debug in chrome as firebug fucks up badly with switch control
    switch (requestbuild.ajaxRequestSubType) {
      //control processes so no actual ajax requests are required
      case "multirequest_parent":
      case "multilrequest_submit":
        break;
      default:
        this.server_data[request_id].ajax = $.ajax({
          headers: {
            'X-CSRF-TOKEN': laravelCSRF//this is a constant from php JSCONSTANTS
          },
          url: this.url,
          type: this.reqType,
          data: requestbuild,
          dataType: this.dataType,
          success: function (server_response) {

            var requestobj = SERVER_INTERFACES[server_response.interface_name]; //.server_data[request_id];
            if (requestobj.server_data[request_id] != undefined) {//check existence - a reset might have been requested since query sent
              requestobj.setRequestResult(server_response.request_id, server_response.data);
              //check through request types to eventual detect a simple callback from client object
              switch (server_response.ajaxRequestSubType) {
                case "select":
                case "call":
                  if (server_response.flag_multiRequestChild) {
                    var parentinterface_name = requestobj.interface_name;
                    var parentinterface_id = server_response.multirequest_parent;
                    var multiobj =
                            SERVER_INTERFACES[parentinterface_name].
                            server_data[parentinterface_id];
                    multiobj.request_returned++;
                    if (multiobj.request_returned == multiobj.request_total) {
                      requestobj.multiRequestFinish(requestobj.server_data[request_id].multirequest_parent);
                    }
                  } else if (server_response.callback != "") {
                    eval(server_response.callback + "(" + server_response.request_id + ",'" + requestobj.interface_name + "')");
                  }
                  break;
                case "submit":
                  if (!server_response.ismultipart) {
                    if (server_response.callback != "") {
                      eval(server_response.callback + "(" + server_response.request_id + ")");
                    }
                  } else {
                    var multiobj = SERVER_INTERFACES[server_response.interface_name].server_data[requestobj.server_data[request_id].multisubmit_parent];
                    multiobj.submit_returned++;
                    if (multiobj.submit_returned == multiobj.submit_total) {
                      requestobj.multiSubmitFinish(requestobj.server_data[request_id].multisubmit_parent);
                    }
                  }
                  break;
                case "jscsv":
                  var downloadobj = SERVER_INTERFACES[server_response.interface_name].server_data[request_id];
                  requestobj.downloadrun(downloadobj);
                  break;

                default://need failover as cannot determine what to do
                  break;
              }
            } else {
              var faildata = "error";
            }
          },
          error: function (server_response) {
            //parse unhelpful error 'data' to relocate correct ajax request object
            var errordata = this.data.split("&");
            var intefacename;
            var requestid;
            var errorobj = {isajaxerror: true};
            for (var i in errordata) {
              var keyval = errordata[i].split("=");
              errorobj[keyval[0]] = keyval[1];
              if (errordata[i].indexOf("request_id=") != -1) {
                requestid = errordata[i].substr(errordata[i].indexOf("=") + 1);
              }
              if (errordata[i].indexOf("interface_name=") != -1) {
                interfacename = errordata[i].substr(errordata[i].indexOf("=") + 1);
              }
            }
            var parentobj = SERVER_INTERFACES[interfacename];
            var requestobj = parentobj.server_data[requestid];
            //new object required as out of scope

            errorobj["responseText"] = server_response.responseText;
            parentobj.setRequestResult(errorobj["request_id"], errorobj);
            eval(errorobj["callback"] + "(" + errorobj["request_id"] + ")");
          }
        });
        break;
    }
    return request_id;
  }

  /*
   * handles ajax where data is not expected back such as an insert statement or email send
   * but can expect a response message such as 'ok' or an error message
   */
  Ajax_m.prototype.submit = function (type, submitdata, callback) {
    this.request({ajaxRequestSubType: "submit", type: type, submitdata: submitdata, ismultipart: false, callback: callback});
  }

  /*
   * 'multiSubmit' handles ajax where data is not expected back such as an insert statement or email send
   * but can expect a response message such as 'ok' or an error message
   * EXAMPLE
   var multisub = [
   {
   type: "update",
   data: {
   table: "[xondbs1].stats.dbo.a_ppuser",
   values: {'email': 'tim'},
   matchfield: 'keyval', matchvalue: 'Q00010017'
   }
   },
   {
   type: "update",
   data: {
   table: "[xondbs1].stats.dbo.a_ppuser",
   values: {'email': 'tim'},
   matchfield: 'keyval', matchvalue: 'Q00010017'
   }
   }
   ];
   ajaxdbobj.multiSubmit(multisub, "callbackfunctionname");
   */
  Ajax_m.prototype.multiSubmit = function (submitobject, callback) {
    var parent_request_id = this.request({ajaxRequestSubType: "multisubmit_parent", submit_total: count(submitobject), submit_returned: 0, id_list: [], submit: {}, response: {}, callback: callback});
    for (var s in submitobject) {
      var child_request_id = this.request({
        ajaxRequestSubType: "submit",
        multisubmit_parent: parent_request_id,
        ismultipart: true,
        type: submitobject[s].type,
        submitdata: submitobject[s].data
      });
      this.server_data[parent_request_id].id_list[child_request_id] = s;
    }
    return parent_request_id;
  }

  /*
   * sets up mutli queries to only run callback when all complete
   * the requestobject is key=>query assoc to which results are assign the same key
   * when all results complete the callback is run giving the request_id in the normal way
   * to return the multi result object
   */
  Ajax_m.prototype.multiRequest = function (requestobject, callback) {
    var parent_request_id = this.request({ajaxRequestSubType: "multirequest_parent", request_total: count(requestobject), request_returned: 0, id_list: [], request: {}, data: {}, callback: callback});
    for (var r in requestobject) {
      /*var child_request = {
       request: requestobject[r],
       ajaxRequestSubType: "multirequest_child",

       }*/
      var child_request = requestobject[r];
      child_request.multirequest_parent = parent_request_id;
      child_request.flag_multiRequestChild = true;
      var child_request_id = this.request(child_request);
      this.server_data[parent_request_id].id_list[child_request_id] = r;
    }
    return parent_request_id;
  }

  /*
   * sets up facility for javascript to download data locally
   * there is no callback facility required as this is a one-way request
   * with a url returned pointing to the resultant file, this is ultimately sent as a new
   * window request to the file which will be handled on the native machine settings
   *
   * for integrity and security the server sets a unique filename ending
   *
   * First a meta query is sent to process what action to take - eg inform that data is emailed when ready if a length query
   * This is then fed back to the user as to what is happening before the actual query can begin.
   * @param type determines what download process should be used
   * @param dataArgs determines how the data is retrieved
   */
  Ajax_m.prototype.requestDownload = function (fileprefix, type, downloadData) {
    view.dialogOpen({
      title: "Download",
      dialogbody: "preparing download . . .",
      closeButton: false//change to false after test
    });
    this.request({ajaxRequestType: "requestDownload", fileprefix: fileprefix, ajaxRequestSubType: type, downloadData: downloadData});
    //this.server_data[downloadid].callbacktype = action;
  }

  /*
   * opens url to processed data downloadRequest in a new window / tab
   */
  Ajax_m.prototype.downloadrun = function (reqobj) {
//getRequestResult
    var meta = this.getRequestResult(reqobj.request_id);
    switch (reqobj.ajaxRequestSubType) {
      case "jscsv":
        view.dialogOpen({
          title: "Download",
          dialogbody: "Your file download has finished processing.<br />Please ensure you have popups enabled for this site to be able to access the file.",
          closeOK: true
        });
        window.open(meta.downloadurl, "download");
        break;
      case "dbcsv":
        view.dialogOpen({
          title: "Download",
          dialogbody: meta.msg,
          closeOK: true
        });
        this.request({ajaxRequestSubType: "downloadrun", filename: reqobj.filename, type: reqobj.type, downloadsource: reqobj.downloadsource}, '');
        break;
    }
  }

  /*
   * kills data (returning requested will be ignored)
   */
  Ajax_m.prototype.reset = function () {
    for (var s in this.server_data) {
      if (this.server_data[s].ajax) {
        this.server_data[s].ajax.abort();
      }
    }
    this.server_data = new Array();
    this.request_index = 0;
  }

  /*
   * relates misc data to query eg
   */
  Ajax_m.prototype.setMisc = function (request_id, miscobject) {
    this.server_data[request_id].misc = miscobject;
  }

  /*
   * gets misc data to query eg
   */
  Ajax_m.prototype.getMisc = function (request_id) {
    return this.server_data[request_id].misc;
  }

  /*
   * get orig query sent
   */
  Ajax_m.prototype.setAjaxRequestType = function (type) {
    this.reqType = type;
  }

  /*
   * get orig query sent
   */
  Ajax_m.prototype.setDataType = function (type) {
    this.dataType = type;
  }

  /*
   * get orig query sent
   */
  Ajax_m.prototype.getRequest = function (request_id) {
    return this.server_data[request_id].request;
  }

  /*
   * return result data for given request id
   */
  Ajax_m.prototype.getRequestResult = function (id) {
    var data;//leave as undefined so code fails in the client
    if (this.server_data[id]) {
      data = this.server_data[id].data;
    }
    return data;
    //return data;
  }

  /*
   * return result data for given request id indexed by a field name
   * if its not unique there will be data loss and a 'distinct' set will be returned
   */
  Ajax_m.prototype.getRequestResultByCol = function (id, colname) {
    var reqdata = this.getRequestResult(id);
    var data = {};
    for (var r in reqdata) {
      data.r[colname] = data.r;
      delete data.r[colname][colname];
    }
    return data;
    //return data;
  }

  /**
   * return a single value for given request id, if request id did actual generate
   * multiple values then only the first is returned
   * if this was a multirequest, the named key of the multi request is required
   */
  Ajax_m.prototype.getRequestValue = function (id, multi_id) {
    var retval;
    if (!this.server_data[id].ismultiparent) {
      retval = this.server_data[id].data[0];
    } else {
      retval = this.server_data[id].data[multi_id][0];
    }
    if (retval == undefined) {
      retval = null;
    }
    return retval;
  }



  /*
   * removes a query and data from memory
   */
  Ajax_m.prototype.deleteRequest = function (request_id) {
    delete this.server_data[request_id];
  }


  Ajax_m.prototype.error = function (serverData, st, ert) {
//all errors should have been handled, but just in case
    var errorReport = "error";
    errorReport += st;
    alert("error");
  }

  /**********************************************************************************************************
   INTENDED AS PRIVATE FUNCTIONS
   **********************************************************************************************************/

  /*
   * sets result data for this instance for given query id - eliminates the need  for eval unknown ajax data
   */
  Ajax_m.prototype.setRequestResult = function (request_id, data) {
    this.server_data[request_id].data = data;
  }

  /*
   * compiles the data from the multi query parts into the array entry referenced by the query_index returned
   * from client code calling the multiquery function. This also allows the required callback with a reference id to this data
   */
  Ajax_m.prototype.multiRequestFinish = function (multirequestid) {
    var requestobj = this.server_data[multirequestid];
    var multidata = {};
    var compactdata;
    for (var i in requestobj.id_list) {
      requestobj.data[requestobj.id_list[i]] = this.getRequestResult(i);
      this.deleteRequest(i);
    }
    eval(requestobj.callback + "(" + multirequestid + ")");
  }

  /*
   * finalises multisubmit and runs callback
   */
  Ajax_m.prototype.multiSubmitFinish = function (multirequestid) {
    var requestobj = this.server_data[multirequestid];
    var multidata = {};
    var compactdata;
    for (var i in requestobj.id_list) {
//requestobj.data[requestobj.id_list[i]] = this.getRequestResult(i);
      this.deleteRequest(i);
    }
    eval(requestobj.callback + "(" + multirequestid + ")");
  }

}
marc_s
  • 732,580
  • 175
  • 1,330
  • 1,459
Datadimension
  • 872
  • 1
  • 12
  • 31
0

There were quite a few repetitions in your code. Contrary to @Datadimension I did not restructure your code completely but was trying to show you ways of parameterizing a few of your expressions and functions:

$(document).ready(function() {
 var labcatsubcat1=
 $("label[for='id_category1'],#id_category1,label[for='id_subcategory1'],#id_subcategory1 ")
    .hide();
 $('select[name=subcategory],select[name=subcategory1]')
    .html('<option value="Not selected" selected disabled>Select Category...</option>');

 // called when category field changes from initial value:
 $('#id_group').change(function() {
   labcatsubcat1.toggle($(this).val()=='premium');
 })

 $('#id_category,#id_category1').change(function() {
        var $this = $(this), postfix=this.id.match(/1*$/)[0];
        if ($this.find('option:selected').index() > 0) {
            category_id = $('select[name=category'+postfix+']').val();
            request_url = '/get_subcategory/' + category_id + '/';
            $.ajax({
                url: request_url,
                type: "GET",
                success: function(data) {
                    $('select[name=subcategory'+postfix+']').empty();
                    $.each(data, function(key, value) {
                        $('select[name=subcategory'+postfix+']').append('<option value="' + key + '">' + value + '</option>');
                    });
                }
            })
        }
 });
 ["keywords,Keywords,100","description,Description,250"].forEach(e=>{
    var[id,nam,len]=e.split(",");
    $("label[for='id_"+id+"']").html(nam+" 0/"+len);
    $('#id_'+id).keyup(function() {
        var charsno = $(this).val().length;
        $("label[for='id_"+id+"']").html(nam+" (" + charsno + "/"+len+")");
    });
 });
});
<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>

Unfortunately, this is not (yet) a working snippet, as you did not post your HTML ...

Just a short explanation regarding postfix: This variable will hold the string "1" or "" depending on, whether the this refers to the #id_category1 or the #id_category DOM-element.

Carsten Massmann
  • 26,510
  • 2
  • 22
  • 43
-1

One thing I'd do is combine the two AJAX requests. You can use multiple selectors for the change handler. Something like $('#id_category', '#id_category1').change(etc.... I'm sure you could find a way to target exactly which one is handling which request. You can use multiple selectors for the other functions as well as per the docs.

If the file is relatively short (~100 - 200 lines) I think you're fine keeping everything all in one place. Otherwise, definitely separate the functions into files and then require them as needed. You can use requireJS for this among others.

aberkow
  • 330
  • 1
  • 10
  • require is php ? - no code and incorrect programming language for an answer ! – Datadimension Feb 05 '17 at 20:20
  • It's php natively (? - I'm fairly new to php) but you can use external JS libraries or tools to do it as well. [Require JS](http://requirejs.org) is one example. Others include [webpack](https://webpack.js.org), [browerify](http://browserify.org/), etc. – aberkow Feb 05 '17 at 20:22
  • The question is "avoid code repetition in jQuery" not php – Datadimension Feb 05 '17 at 20:23
  • Require is available in JS using tools as above. Also, I pointed the OP to the jQuery docs regarding multiple selectors. – aberkow Feb 05 '17 at 20:24