2
\$\begingroup\$

I am using the code below that loops around thousands of time, I need this to be as fast as possible, however I am not an expert in Javascript and all I know is that this code is faster than doing it with the innerHTML method.

Can it be optimized? If so, how?

    var a,b,c,d,e,
    trEl, tdEl, imgEl

    for (i = 0; i < docs.length; i++) {
      trEl = document.createElement('tr');
      tdEl = document.createElement('td');
      td1El = document.createElement('td');
      td2El = document.createElement('td');
      td3El = document.createElement('td');
      imgEl = document.createElement('img');

      var str
      a = docs[i]['a']
      b = docs[i]['b']
      c = docs[i]['c']
      d = docs[i]['date'] + " - " + docs[i]['timestamp']
      e = docs[i]['_e']

      trEl.setAttribute('id', e);
      trEl.setAttribute('ondblclick', 'scan(' + '"' + e + '"' + ')');
      imgEl.setAttribute('src', 'css/bullet.png');
      tdEl.appendChild(imgEl);

      tdEl.appendChild(document.createTextNode(a));
      td1El.appendChild(document.createTextNode(d));
      td2El.appendChild(document.createTextNode(b));
      td3El.appendChild(document.createTextNode(c));

      trEl.appendChild(tdEl);
      trEl.appendChild(td1El);
      trEl.appendChild(td2El);
      trEl.appendChild(td3El);

      document.getElementById("search-results").appendChild(trEl)
      }
\$\endgroup\$
2
\$\begingroup\$

Don't evaluate docs.length in every iteration - it's constant so it only needs to be evaluated once:

for (var i = 0, n = docs.length; i < n; ++i)

Deference docs[i] into an aliased variable so that you're not repeatedly doing an array lookup into the same element, and don't use strings for keys unless the key is variable:

var doc = docs[i];
var a = doc.a;
...

If the above local variables a through e are only used once, don't bother storing them at all, just put them in as trivial parameters when needed (see below)

Use DOM properties instead of setAttribute function calls when you can:

trEl.id = doc._e;                // instead of 'e'
imgEl.src = 'css/bullet.png';

Don't use inline event handlers:

trEl.addEventListener('dblclick', scan, false);

There's no need to pass the variable e to scan since it's just the clicked element's ID and therefore trivially extracted within the event handler:

function scan(event) {
    var e = this.id;
    ...
}

Use a DocumentFragment to store all of the new elements, and then add the entire fragment into the document in one go, allowing the browser to handle all of the visible DOM changes in one hit:

var rows = document.createDocumentFragment();

for (...) {
    ...
    rows.appendChild(trEl);
}

document.getElementById("search-results").appendChild(rows);
\$\endgroup\$
  • \$\begingroup\$ @FiatPax I'd be interested to hear how much performance gain you achieve \$\endgroup\$ – Alnitak Jan 7 '15 at 16:51
  • \$\begingroup\$ Here is the report from benchmark.js "optimized x 101,487 ops/sec ±6.66% (56 runs sampled), old x 64,893 ops/sec ±11.52% (64 runs sampled) Fastest is optimized." \$\endgroup\$ – Fiat Pax Jan 7 '15 at 18:21
  • \$\begingroup\$ I applied all of the methods above except one with event listeners, because if I add trEl.addEventListener('dblclick', scan, false); it will complain its undefined. I'm not sure where to add that code. \$\endgroup\$ – Fiat Pax Jan 7 '15 at 18:22
  • \$\begingroup\$ That'll depend on where (and how) you've declared the scan function. \$\endgroup\$ – Alnitak Jan 7 '15 at 18:32
  • \$\begingroup\$ or is it complaining about addEventListener being undefined (older MSIE versions), in which case you could just do trEl.ondblclick = scan \$\endgroup\$ – Alnitak Jan 7 '15 at 18:33

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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