Friday 3 February 2012

InnerHTML : The biggest source of Ugly & Unreadable JS

If tomorrow browsers stopped supporting the innerHTML property , most of the worlds Javascript code would stop working except possible the "hello world" programs :P, But this can't hide the fact the worst JS written is mostly because of the overuse of innerHTML .
Problems :
 -> HTML inside JS file , extremely bad for maintainability
 -> Can overwrite existing data by mistake
 -> Can introduce invalid html into the page which would break the DOM tree practically ensuring that most of the JS in the page has bugs or does not work
 Example
    Given we need to add a person to list from a combobox similar to how you add people on facebook message or in email sender in yahoomail


    It could be acheived in many ways :
    a) When an option is selected in the combobox , we could get the user details construct the div element inside the javascript and append it to the box
    Something similar to this :
        element.onselect=function(){
// getData() gets a JSON object containing data of selected user 

            var data =getData(element.selectedIndex);
            var innerHtml="<div class='containBox'><span class='user'>"+ data.Name+"</span></div>";
            this.innerHTML=this.innerHTML+innerHtml;
        }
    Disadvantages :
    -To change DOM structure we need to access the javascript function
    -String gets bigger and harder to maintain as the widget functionality increases    
    -Population of data is cumbersome with contant appending
    -Easy to introduce bugs since html cannot be validated inside JS strings
   
    Alternative Approach  1 :
    b) In this approach we first create a template HTML copy the DOM element and then append it to the node, then we populate the data as required .
    Example :
        Append the template container as such in an HTML file (include the HTML file wherever required using server side script or dojo xhrGet )
        <div id="containBoxTemplate" class="templateContainer" style="display:none;">
            <div class='containBox'>
                <span class='user'></span>
            </div>
        </div>
       
        The above code can be now rewritten as :
        element.onselect=function(){
// getData() gets a JSON object containing data of selected user 

            var data =getData(element.selectedIndex);
            var innerHtml=dojo.byId("containBoxTemplate").innerHTML;           
            this.innerHTML=this.innerHTML+innerHtml;
            this.lastChild.innerHTML=data.name;       
        }
    Advantages :    
    -HTML template is easier to maintain
    -Probability of bugs are significantly reduced
   
    Disadvantages :
    -Data population is still cumbersome
   
    Alternative Approach 2 :
    c) In this approach we will programattically create all the elements and then populate data as required then append it
        element.onselect=function(){
// getData() gets a JSON object containing data of selected user 

            var data =getData(element.selectedIndex);
            var div = document.createElement("div");
            var span= document.createElement("span");
            span.className="user";
            span.innerHTML=data.name;
            div.appendChild(span);
            this.appendChild(div);           
        }
    Advantages :
    -Chance of accidental deletion of previous content is removed since we are not playing around with innerHTML of parent element
    -No Strings used , clear programmatic structure
    -Efficient
   
    Disadvantages :
    -Complex code
    -Hard to change the HTML template (DOM structure)
   
    While approach c) seems programatically correct , it makes for some really complex and unmaintainable code. It is a pure JS solution where it is not required.
   
    Alternative Approach 3 :
    d) In this approach we will clone the template ,populate the data and then append it
    Now Given the same HTML template ,the code can be rewritten as follows  :
    element.onselect=function(){
// getData() gets a JSON object containing data of selected user 

            var data =getData(element.selectedIndex);
            var cloneNode=dojo.clone(dojo.byId("containBoxTemplate"));
           
            //Query the cloned node only and populate data
            dojo.query(cloneNode).query("span.user")[0].innerHTML=data.name;
           
            this.appendChild(cloneNode);
        }
    Advantages :
    -Code is safe since it does not append to the DOM tree until it is fully populated 
    -Cannot affect existing data since we do not use innerHTML
    -Depends on template and does not modify it ( otherwise that would introduce bugs :) )
   
    Disadvantages :
    Can't think of any - well that would be too patronizing, i think this should be it
    -Slightly complex code 

End of the day none of the techniques are defacto standards to create new DOM elements (or widgets as we like to call them),but a combination of any of the alternative 3 techniques should provide a good combination of advantages and simple code.I would personally use option d) ( no wonder i couldn't think of disadvantages) but any suggestions and better practice would be duly appreciated.Please do share what you practice and would prefer in the above mentioned scenario.
   

Thursday 2 February 2012

dojo.query -The Magical DOM Selector and Best practices

One of the most powerful tool which is a quintessential part of any javascript library is the DOM selector function in the library .
Given an HTML snippet as

<div id="menuList">
    <span class="sub">sm Item</span>
    <span class="sub active">
        <span class="superSub">smthing more </span>       
    </span>
    <span class="sub">sm Item</span>
    <span class="sub">sm Item</span>
</div>

Normally in simple javascript the way to get the DOM element would be something like this :
var menu=document.getElementById("menuList");
//Get sub elements
var subItems=menu.childNodes;

Then you run a loop on the subItems to check if it is an Element or Text node and then apply the busines logic :
for( var i =0 ;i<subItems.length;i++){
    if(subItems[i].nodeType==document.ELEMENT_NODE){
        //Do something with the node
    }
}

Now the same thing using Dojo would be
    dojo.query("#menuList > span").forEach(function(x){
        //Do something with this
    });

Test 1:
        Usecase :     Modify sub menu items under id="menuList"
        Features Used : dojo.query with forEach on it
Well that drastically simplifies the code and makes it more readable , but that is just the tip of the  iceberg
Test 2 :
    Usecase : Create a function which takes the index of the sub menu item to make active ( i.e add the class active to it ) . There can be only one active item

Simple Javascript :
-First we need to get all the span elements under the menu in the same way
 -then check for the class name , remove the class active without affecting the class="sub" which is already there
 -add the class active for the required inde
Code :
        var subItems=menu.childNodes;
        for( var i =0 ;i<subItems.length;i++){
            if(subItems[i].nodeType==document.ELEMENT_NODE){
                subItems[i].className=subItems[i].className.split("active").join("");           
                if(i==INDEX)
                    subItems[i].className=subItems[i].className+" active";
            }
        }   

Result :
Achieved with relative ease but with extremely inefficient (who cares about efficiency in JS ! :) ) string functions .Biggest issue is the possibility of bugs in the logic if the string "active" instead of " active" , the code would have failed.More possibilities of bugs are there in a relatively very common activity .
   
Dojo :
Similar procedure :
Imagine doing the same thing with dojo.addClass and dojo.removeClass .It makes it much more easier .
2 Ways :
        Way 1 :
            var i=0;
            dojo.query("#menuList > span").forEach(function(x){
                if(i!=INDEX)
                    dojo.removeClass(x,"active");               
                else
                    dojo.addClass(x,"active");               
                i++;
            });

           
        Way 2 :
  But the magic of dojo.query is that most DOM utility functions can be applied on returned array directly.So , Best practice :
      dojo.query("#menuList > span").removeClass("active");
   dojo.addClass(dojo.query("#menuList > span")[INDEX],("active"));

    Result : 
Well,thats 2 lines of code with not a for loop in sight (ok ,its inside the function ) and which is more intutive,readable and easier to maintain.The possibility of bugs are also reduced to a great extent .   
Now in the same way we could do various things like :
    -> dojo.query(".....").style(PropertyName,NewValue)
    -> dojo.query(".....").attr(PropertyName,NewValue)  - Especially useful when we set properties in HTML as custom attributes
    -> ofcourse dojo.query(".....").addClass , dojo.query(".....").removeClass , dojo.query(".....").toggleClass
    -> Attach EventListeners - the most useful aspect

    Example Usecase : Onclick of any submenu item should make them active and rest inactive  :
    Code :
            dojo.query("#menuList > span").onclick(function(){
                    dojo.query("#menuList > span").removeClass("active");
                    dojo.addClass(this,"active");
            });

 This leads to much cleaner code than mixing the onclick with the html and calling a function .
   
The Problem of the unique ID :
Normally when we initially start writing JS we tend to assign ID's in too many places.Every DOM element that we need gets assigned an ID adn we directly get that element and work with it in our JS function -animation , validation e.t.c.
But We soon run up into the following issues :
 -> Run out of unique ID's - For someone like me used to variable names like "kk" , "i" and "asd" ("foo" for many others) , the ID's start getting longer and
 the chance of duplicity increases and with it unknown bugs
 -> May not have control of the JSP ( or anyother thing for that matter ) for which the JS is to be written which means that we need to contantly trouble the other guy to add Id's or do code merges ( god save your code when merging it )
 -> Duplicity of code , we might not see the generalization of business logic and end up writing duplicate code for what could be essentially the same thing

Well , the best practice would be to seperate discrete UI components like Top Menu , Body ,Status Bar e.t.c with discrete ID's and tag similar UI compontents with similar class name.
Then, the DOM element which needs to be selected should be queried according to the DOM structure .
Example :
Given the snippet
<body>
    <div id="menuList">
        <span class="sub">sm Item</span>
        <span class="sub active">
            <span class="superSub">smthing more </span>       
        </span>
        <span class="sub">sm Item</span>
        <span class="sub">sm Item</span>
    </div>
    <div id="contentbody">
        ...
    </div>
    <div id="commentContainer">
        <div class="comment">....</div>
        <div class="comment">....</div>
        <div class="comment">....</div>
    </div>
    <div id="footer">
    </div>
</body>

   
In this way the HTML should have as few Id's as possible so that CSS and JS are lot simpler and generalized to avoid duplicity of codes which directly reduces the number of bugs (lesser work for the M&E team - seems bad for me in the long run  :) ) . Especially now that we have dojo.query , it is much better to stop avoiding the usage of a thousand id's and rather an expression to get to the DOM element .
Something like :
dojo.query("#idName class1 tag1.className2 > span")
and then use it to implement JS logic .