Showing an item in a shopping cart

Question

I'm updating and old project from jQuery 1.4 to jQuery 1.7.

Can this be simplified, thus reducing the amount of code, and perhaps in such reduction, have it improved?

Having the following code:

var cartItemDel   = '<td class="del"><img src="components/themes/default/img/icons/cross-small.png" width="16" height="16" alt="X"><input type="hidden" class="lid" name="lineID'+ id +'" value="'+ id +'"><span class="hidden token">'+token+'</span></td>',
    cartItemName  = '<td><input class="CartItemId" type="hidden" name="id_'+ id +'" value="'+ id +'"><span class="CartItemName">'+ name +'</span></td>',
    cartItemPrice = '<td><input class="CartItemRef" type="hidden" name="ref_'+ ref +'" value="'+ ref +'"><span class="class="CartItemPrice"><span class="lineSum">'+ price +'</span>&euro;</span></td>';

$(".cart-table tbody")
    .append('<tr>'+cartItemDel+cartItemName+cartItemPrice+'</tr>');

I've updated it to:

/*
 * ADD NEW LINE - First TD
 */
var $newLineIcon = $("<img/>", {
        src    : "components/themes/default/img/icons/cross-small.png",
        width  : "16",
        height : "16",
        alt    : "X"
    });

var $newLineLID = $("<input/>", {
        type  : "hidden",
        class : "lid",
        name  : "lineID"+ lid,
        value : lid
    });

var $newLineToken = $("<span/>", {
        class : "hidden token",
        text  : token
    });

var $newLine_firstTD = $("<td/>", { class : "del"})
    .append($newLineIcon)
    .append($newLineLID)
    .append($newLineToken);

/*
 * ADD NEW LINE - Second TD
 */
var $newLinePID = $("<input/>", {
        type  : "hidden",
        class : "CartItemId",
        name  : "id_"+ id,
        value : id
    });

var $newLinePNAME = $("<span/>", {
        class : "CartItemName",
        text  : name
    });

var $newLine_secondTD = $("<td/>")
    .append($newLinePID)
    .append($newLinePNAME);

/*
 * ADD NEW LINE - Third TD
 */
var $newLinePREF = $("<input/>", {
        type  : "hidden",
        class : "CartItemRef",
        name  : "ref_"+ ref,
        value : ref
    });

var $newLinePPRICE = $("<span/>", {
        class : "CartItemPrice",
        html  : "&euro;"
    })
    .prepend($("<span/>", {class : "lineSum", text : price}));

var $newLine_thirdTD = $("<td/>")
    .append($newLinePREF)
    .append($newLinePPRICE);

$("<tr/>")
    .append($newLine_firstTD)
    .append($newLine_secondTD)
    .append($newLine_thirdTD)
    .appendTo($(".cart-table tbody"));

Answer 1

+1 to this code looking fine as is (with caveats and one issue).

Issue: You should always quote class as it is a reserved word for future use in javascript.

Don't take this as me advocating you use it or saying it is ready for production¹, but I'm using JsRender in production to ease much of the pain you are seeing/imagining here.

I think the central issue here is that this type of Javascript invokes a hard dependency on the structure of the page and so we look at it with a bit of queasiness that it rightly deserves. If your visual designer comes along and decides that the shopping cart needs to be in a dropdown list where you can drag (or swipe) items out of or into now you have to go and make a whole bunch of changes all over the place.

Unfortunately I don't think it is reasonable in a modern website to expect both that the html has no dependencies on the Javascript ("page works without javascript") and that the Javascript has no dependencies on the design of the html("javascript works equally well on a totally different design that happens to have a couple of the same named elements").

I personally am comfortable ignoring the former to get much closer to an ideal on the latter. That said, my working environment is such that I have very little control over the redesigns that happen all too often in the products I work on.

At a minimum I would strive to make sure this code stayed together inside its own function at or near the top of your javascript (or in its own global/namespaced function in the html file itself). This code has very little to do with your logic and a lot to do with your site design. Therefore it doesn't belong sitting alongside your page logic.

On minification (for this code in particular) I am less concerned. A solid minifier is going to combine your var statements, rename the variables and possibly inline them and then remove whitespace. If you were concerned about this, you could inline everything yourself (and you would lose a little on the readibility), but the gzip of this even uncompressed is only about 480 bytes (closure compiler adds some stuff before it estimates) so at most you are going to gain maybe 100 bytes (closure advanced gets 140 but the result will not work; I can manually do it and get 145 off; none of these numbers will mean anything in terms of page performance for your site).

FWIW, your original code was only 332 bytes zipped (smaller than my manual attempt) and compressed (manually) down to 297.

---

1. The tests are poorly covering the functionality (willing to bet there are significant issues in various places) and it is sorely missing even half decent documentation.

Answer 2

Your code looks OK to me, very clear and organised.

It's more important to minify your code to decrease load time. There's a whole bunch of minification tools on the Internet.