2
\$\begingroup\$

First of all, I am not using JQuery but Cheerio. I have never actually used JQuery in "real life" -- I started with Cheerio, which is a subset and doesn't actually work in a browser environment.

(Note: I don't have enough reputation here to create tags, somebody might want to tag this question as "cheerio" and delete this sentence...)

Here is my code. Questions are inline:

$ = cheerio.load( self.formPage.toString() );

$( 'form' ).each( function( index, form ){

  // *****************
  // QUESTION:
  // Is this the right way to do this?
  // I am "converting" each found "DOM" object into a "JQuery" object (although
  // it's not real DOM, and it's not real JQuery either).
  // But, if there is no ID set for form, for example, is this _guaranteed_ to always find
  // the right one?
  var $form = $( form );

  // Work out fullAction, which is the form's action relative to the fetched URL
  var action;
  var baseUrl = self.config.url.substr(0, self.config.url.lastIndexOf("/") + 1);
  var action = $form.attr( 'action' );
  var fullAction = require( 'path' ).normalize( baseUrl + action );
  console.log("FULL ACTION:", fullAction );

  stores.workspacesNinjaFormCodes.apiGetQuery( { filters: { workspaceId: self.workspaceId }, skipHardLimitOnQueries: true }, function( err, codesData ){

  //stores.workspacesNinjaFormCodes.dbLayer.select( { conditions: { and: [ { field: 'workspaceId', type: 'eq', value: self.workspaceId } ] } }, { skipHardLimitOnQueries: true }, function( err, codesData ){

    codesData.forEach( function( codeData ){
      var code = codeData.code;
      var dataSubmitHash = {};

      // *****************
      // QUESTION:
      // Is this the right way to do this? I want to find all 'input' fields 
      // in this particular form. So, I pass $form as context and look for
      // all 'input' tags. Is this what you are meant to do?
      // ******************
      $( "input", $form ).each( function( index, inputField ){

        // *****************
        // QUESTION:
        // Once again, will this always work, regadrless of IDs etc.? I am passing `form` as the
        // context.
        // ******************
        var $inputField = $( inputField, form );

        // See if the field name is already assigned
        var fieldValue = $inputField.val() ? $inputField.val() : code;

        // Assign values
        dataSubmitHash[ $inputField.attr('name') ] = fieldValue;
      });


      console.log("dataSubmitHash:", dataSubmitHash );
    });

  });

});

Questions are inline, and they are focused on this: how expensive is the "conversion" of cheerio's "DOM" objects into Cheerio "JQuery" objects? And is it always guarantee to get the right one?

\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

Here's what I did, comments in the code.

$ = cheerio.load(self.formPage.toString());
path = require('path'); //Move out path from the loop to avoid re-requiring it
$('form').each(function (index, form) {

  var $form = $(form);

  // Removed action here. You seem to use it after the next line.

  var baseUrl = self.config.url.substr(0, self.config.url.lastIndexOf("/") + 1);
  var action = $form.attr('action');
  var fullAction = path.normalize(baseUrl + action);

  // In the next block, input is in the context of this form
  // We can pull it out of the loop to avoid fetching it on every iteration
  var input = $('input', form);

  // You don't seem to use fullAction in the next block?

  stores.workspacesNinjaFormCodes.apiGetQuery({
    filters: {
      workspaceId: self.workspaceId
    },
    skipHardLimitOnQueries: true
  }, function (err, codesData) {
    codesData.forEach(function (codeData) {
      var code = codeData.code;
      var dataSubmitHash = {};

      // here's the input earlier
      input.each(function (index, inputField) {

        // Let's keep it readable, place then in variables
        var $inputField = $(inputField, form);
        var inputFieldName = $inputField.attr('name');

        // we could use the || operator to check for the first and use it, or use the second if not first.
        var fieldValue = $inputField.val() || code;

        dataSubmitHash[inputFieldName] = fieldValue
      });

      console.log("dataSubmitHash:", dataSubmitHash)
    })
  })
});

In addition, one should not create functions inside loops. Consider moving them out of the loops.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.