Refactor function that returns a comma Operator to pass jsHint

Question

Some background

This functions does some business logic before submitting a "add to cart" form

HTML

<form class="atcLoginRequired" method="get" action="/somePage">
    <input type="hidden" value="someVal" name="nextpage">
    <input name="ISODSCTX66WA_qty" type="text" value="1">
    <input name="ISODSCTX77WA_qty" type="text" value="1">
    <input type="Image" class="imageSubmit" src="ATCimage.jpg">
</p>

JavaScript

 var atcLoginRequired = $(".atcLoginRequired");
    if (!atcLoginRequired.length) return;
    atcLoginRequired.on("submit", function(){
        var jThis = $(this),
            nextPageInput = jThis.find("[name='nextpage']"),
            nextPageVal = nextPageInput.val(),
            skuValues = jThis.find("[name$='_qty']").map(function(){
                var jThat = $(this);
                return (jThat.remove(), jThat.attr("name") + "=" + jThat.val() + "&"); 
              // the return statement actually executes the first expression, and "returns" the second one..  
            }).get().join("");
        $(".imageSubmit").remove(); // to avoid having the x and y passed
        skuValues = skuValues.substring(0, skuValues.length - 1);
        nextPageInput.val(nextPageVal + "" + skuValues);
    });

The question

• What can I do to make this it pass jsHint?
• What can I do to make this better in terms of readability, performance and extensibility ?

Answer 1

Simple answer:

var jThat = $(this).remove();
return jThat.attr("name") + "=" + jThat.val() + "&"; 

There's nothing wrong with calling .remove() right away; the element's still in memory so its attributes be still be accessed. As you said, the remove call is already executed in your code, before the rest.

Longer answer:
Don't write your own query string by concatenating stuff with "&" and "=". It's error-prone, a bother to deal with, and it also means you have to do the slice off the trailing "&" later (which, incidentally, you can do with just skuValues.slice(0, -1)).

jQuery has a $.param() function that's built for this sort of thing. Pass it an array of objects, each with name and a value property, and it'll make a properly encoded query string for you.

// no need to first store `$(".atcLoginRequired")` in a variable,
// and then check its length. If nothing was found, jQuery will
// just ignore the call to .on(). Same result as you checking, and
// and returning if length is zero.

$(".atcLoginRequired").on("submit", function (event) {
  var form          = $(this), // use a more descriptive name than "jThis"
      nextPageInput = form.find("[name='nextpage']"),
      skuValues     = form.find("[name$='_qty']").map(function () {
        var input = $(this).remove(); // again, more descriptive
        // return an object fit for jQuery's param() function
        return {
          name:  input.attr("name"),
          value: input.val()
        };
      }).get();

  // add the nextPage stuff, if there is any
  if( nextPageInput.val() ) {
    skuValues.push({
      name: "nextpage",
      value: nextPageInput.val()
    });
  }
  nextPageInput.val( $.param(skuValues) );
});

Overall, though, I'm wondering what the point is of stuffing a big query string into a hidden input. That's going against the grain. Why not send the inputs to the server as-is, and let it add a bunch of hidden inputs to the next page's form or something? Using an input to store other inputs seems like a kludge.

---

What can I do to make this better in terms of readability, performance and extensibility ?

For readability, see above. Less code, more descriptive variable names. You can find better names than mine - I just went for simple ones.

Performance... well, there isn't a whole lot going on. It's not a complex algorithm. If you want performance, skip jQuery and go straight to the native DOM. Of course, that's a hassle to code, which is why jQuery is extremely useful.

Extensibility: In what way exactly? This is, I presume, not a library in itself. It's the end-goal code. So extensibility is sort of moot. But if you want to add more on-submit functionality (like, say, form validation), the first order of business is to not remove elements. If you do that, the next piece of code will just find an empty form. Validation would be sort of pointless if it comes after this code, as it's not going to find anything but a hidden input with a big query string as its value. And if you need to validate that input, you'll need to parse that string, just to get back to the individual values again.