- From: malcolm tent <tmalcolm417@gmail.com>
- Date: Sat, 7 Aug 2021 18:27:10 -0700
- To: site-comments@w3.org
- Message-ID: <CAEcE9Mi1mfu0JQbSJCQKdd3ORB_-L5OpsHh4atweCc7fvNF1YQ@mail.gmail.com>
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 ‘ (‘) and ’ (’) 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