W3C home > Mailing lists > Public > site-comments@w3.org > August 2021

Comments regarding //www.w3.org/wiki/Javascript_best_practices

From: malcolm tent <tmalcolm417@gmail.com>
Date: Sat, 7 Aug 2021 18:27:10 -0700
Message-ID: <CAEcE9Mi1mfu0JQbSJCQKdd3ORB_-L5OpsHh4atweCc7fvNF1YQ@mail.gmail.com>
To: site-comments@w3.org
Comments regarding page (with localised fragment reference)
//www.w3.org/wiki/Javascript_best_practices#Avoid_heavy_nesting

There are several issues regarding the code samples provided.
1) They include lexically invalid characters, specifically ‘ (&lsquo;) and
’ (&rsquo;) used where only (paired) " or ' are valid. This makes it
difficult to copy the examples for local testing and could give the (false)
impression to novices that such characters are legal, leading to confusion
regarding the reason for the compiler complaints (It took a second look to
interpret the "missing ) error" generated by
  var out = document.getElementById(‘profiles’);
). Other samples in the article are correct, all using '. (I note that the
original on github does not have this problem.)

2) Sample 2 contains a typo:
    li.appendChild(document.createTextNode(data.members[i].name));
should be
    li.appendChild(document.createTextNode(o.members[i].name));

3) The samples are semantically incorrect as follows:
a) The ul node (in variable 'ul') in renderProfiles (both samples) has no
child nodes assigned.
b) Even if the created li nodes (in 'li') were appended to the (presumably)
enclosing ul node (in 'ul'), the fact that 'ul' is reassigned for every
element of o.members means that only the last generated ul and li nodes (in
'ul' and 'li' upon loop exit) would become attached to the document tree,
the rest ending up unreferenced in the garbage to be collected.
c) In addMemberData (sample 2), the li node is appended to the ul node
outside the loop so only the last generated li node becomes attached to the
document tree.

Sample 1:
function renderProfiles(o){
  var out = document.getElementById(‘profiles’);
  for(var i=0;i<o.members.length;i++){
    var ul = document.createElement(‘ul’);
    var li = document.createElement(‘li’);
    li.appendChild(document.createTextNode(o.members[i].name));
    var nestedul = document.createElement(‘ul’);
    for(var j=0;j<o.members[i].data.length;j++){
      var datali = document.createElement(‘li’);
      datali.appendChild(
        document.createTextNode(
          o.members[i].data[j].label + ‘ ‘ +
          o.members[i].data[j].value
        )
      );
      nestedul.appendChild(datali);
    }
    li.appendChild(nestedul);
  }
  out.appendChild(ul);
}

Sample 2:
function renderProfiles(o){
  var out = document.getElementById(‘profiles’);
  for(var i=0;i<o.members.length;i++){
    var ul = document.createElement(‘ul’);
    var li = document.createElement(‘li’);
    li.appendChild(document.createTextNode(data.members[i].name));
    li.appendChild(addMemberData(o.members[i]));
  }
  out.appendChild(ul);
}
function addMemberData(member){
  var ul = document.createElement(‘ul’);
  for(var i=0;i<member.data.length;i++){
    var li = document.createElement(‘li’);
    li.appendChild(
      document.createTextNode(
        member.data[i].label + ‘ ‘ +
        member.data[i].value
      )
    );
  }
  ul.appendChild(li);
  return ul;
}

4) As a comment regarding programming style, in particular "information
hiding", it could be an improvement in Sample 2 to pass only the necessary
data to addMemberData instead of the entire member object:

  function renderProfiles(o){
  ...
    li.appendChild(addMemberData(o.members[i].data));
  ...
}

  function addMemberData(data){
  var ul = document.createElement(‘ul’);
  for(var i=0;i<data.length;i++){
    var li = document.createElement(‘li’);
    li.appendChild(
      document.createTextNode(
        data[i].label + ‘ ‘ +
        data[i].value
      )
    );
    ul.appendChild(li); // these 2
  }                     // lines swapped
  return ul;
}

This also reduces the typing and hence the risk of typos. (Note: included
fix for per iteration appending of li node.)


Continuing into "Optimize loops", the advice to avoid dereferencing an
object member (often Array.length) for every loop iteration seems sound but
is likely irrelevant these days as most compilers would probably be able to
optimise away such dereferences if they could determine that the value was
not changed during the loop execution. Further, even in the absence of such
optimisations, this advice is mostly applicable for tight loops on large
objects (the relative benefit when applied to large loops or small objects
is reduced). Also, this advice is not generally applicable as some loops
might alter the size of the array by adding or deleting elements
(admittedly this must be done with great care regarding the effect on the
validity of the iterator). The advice might still be useful if most
compilers choose to assume that mutable objects can change due to
asynchronous events and so no optimisations can be performed, although the
programmer would also have to be aware of this possibility otherwise
implementing the advice would create the same problem the compiler was
avoiding.
Received on Monday, 9 August 2021 08:01:43 UTC

This archive was generated by hypermail 2.4.0 : Monday, 9 August 2021 08:01:44 UTC