I kind of feel like I'm abusing the DOM with my code...

for(var i=0; i<json.length; ++i){       
    var newobj = document.createElement("div");
    var inner = "<img src='"+json[i].picture+"' alt='Item preview' class='icon' />";

    inner += "<p>"+json[i].id+"<br />qty:"+json[i].qty;
    inner += "<a href=\"javascript:clearProduct('"+json[i].id+"')\">";
    inner += "<img alt='Seperator' src='images/form-sep.gif' />";
    inner += "<img src='images/cross.png' alt='Remove this item.' title='Remove this item.' />";
    inner += "</a></p>";
    newobj.innerHTML = inner;
    newobj.className = "cart_item";
    $('cartitems').appendChild(newobj);
    $('cartcount').innerHTML = json.length;
}

Is there a better way to do this? I mean, yes I could through and use createElement for each element and setting each attribute separately but that seems like a lot just when this is so simple. Is there a better way?

I kind of feel like I'm abusing the DOM with my code...

for(var i=0; i<json.length; ++i){       
    var newobj = document.createElement("div");
    var inner = "<img src='"+json[i].picture+"' alt='Item preview' class='icon' />";

    inner += "<p>"+json[i].id+"<br />qty:"+json[i].qty;
    inner += "<a href=\"javascript:clearProduct('"+json[i].id+"')\">";
    inner += "<img alt='Seperator' src='images/form-sep.gif' />";
    inner += "<img src='images/cross.png' alt='Remove this item.' title='Remove this item.' />";
    inner += "</a></p>";
    newobj.innerHTML = inner;
    newobj.className = "cart_item";
    $('cartitems').appendChild(newobj);
    $('cartcount').innerHTML = json.length;
}

Is there a better way to do this? I mean, yes I could through and use createElement for each element and setting each attribute separately but that seems like a lot just when this is so simple. Is there a better way?

• javascript
• ajax
• json
• prototypejs

• 2 Are you using a library? Or is the $ function your own? – Sasha Chedygov Commented Jun 28, 2009 at 7:38
• 1 I am using the prototype library. – MackDaddy Commented Jun 28, 2009 at 7:44
• Oughtn't you use var to declare your variables, to limit their scope to the loop? – Andrew Johnson Commented Jun 28, 2009 at 8:15
• sorry, yes I am using them, I just removed them to try to get the formatting correct here. – MackDaddy Commented Jun 28, 2009 at 8:17

6 Answers 6

I like to create a nice element-creation interface; something like this:

function el( name, attrs ) {

    var elem = document.createElement(name),
        styles = attrs.style;

    for (var prop in styles) {
        try {
            elem.style[prop] = styles[prop];
        } catch(e) {}
    }

    for (var attr in attrs) {
        if (attr === 'style') { continue; }
        elem[attr] = attrs[attr];
    }

    return elem;

}


var myImage = el('img', {
    src: '/path/to/image.jpg',
    alt: 'Really cool image',
    style: {
        border: '1px solid red',
        padding: '10px'
    }
});

I don't know prototype, but I usually build my objects in jQuery like this :

$("<p>").append(
  $("<img>")
  .attr("alt", "Sperator")
  .attr("src", "images/form-sep.gif")
).append(
  $("<img>")
  ... etc ...
)

You can use document.createElement() to create every element in your DOM tree - but this would indeed be way more clumsier (although arguably more "correct"). Not to mention WAY slower on IE. I've run into this problem before - DOM operations are extremely sluggish for IE. Assigning a whole bunch of elements via innerHTML is several orders of magnitude faster for it. Of course, you won't feel it if you're creating just a few elements. But for a 10x300 table it takes several seconds (more than 10 on my old Sempron 2200+ PC).

Whenever I am doing plicated element creation via JS - I normally use Prototypes interpolate method.

To do this, create a template in HTML for the element you want to create and put it on the bottom of your page with display:none so it is hidden. Then use Prototype's interpolate method to replace the specific fields with your content: http://www.prototypejs/api/string/interpolate and call show() on the html to make it visible.

I never use innerHTML. If you try and append strings to innerHTML and then later refer to the DOM objects, you can get into trouble in IE. I could use innerHTML safely in many cases, but I see no reason to. My preferred idiom is to create each DOM object, and then append text as follows:

var div = document.createElement('div');
div.appendChild(document.createTextNode('This is the inner text'));

However, I use some abbreviations, so my code actually would look like this:

var div = dec('div');
div.appendChild(dct('This is the inner text'));

I would like to abbreviate the appendChild() method as well, since that's such a mon JavaScript method, but IE also plains if you try and monkey-patch the DOM by creating a .ac() method.

TBH, unless you're using a pre-rolled library (or are OCDish enough write your own), you're probably best off using innerHTML for as long as IE<8 is a significant factor (which, given the inertia of corporate IT, will be a long time). The (using the term loosely) DOM implementation in IE prior to IE8 is so buggy, non-standard and inplete that any at least usably plete code will be littered with conditionals and special cases.

FWIW, I've found it clearest to build HTML from a data denotation, with the code that translates it as far as possible assuming a working DOM, with post-hoc fixup for IE done with code that's only loaded for broken browsers.

Here's what your code would look like:

horus.appendChild
  ('cartitems',
   [ 'div', '.cart_item',
     [ 'img', { src: json[i].picture, alt: 'Item preview', classname: 'icon' } ],
     [ 'p', null,
       json[i].id, [ 'br' ], json[i].qty,
       [ 'a', { href: 'javascript:clearProduct('+json[i].id+')' },
         [ 'img', { alt: 'Seperator', src: 'images/form-sep.gif' } ],
         [ 'img', { src: 'images/cross.png', title: 'Remove this item.' } ] ] ] ]);

horus.childText('cartcount', json.length);

I'm not going to post our libraries here in their entirely, but have a look in here (starting at line 574, with the meat at 743) if you're curious, with the IE fixup code in here.