Friday, February 8th, 2008

Refactoring your JavaScript for fun and profit

Category: JavaScript

Our newest Ajaxian Chris Heilmann – London Yahoo! – has written a piece on his blog covering five things to do to a script before handing it over to the next developer.

He walks through a refactoring experience:

Let’s say the job was to add small link to every DIV in a document with the class collapsible that would show and hide the DIV. The first thing to do would be to use a library to get around the issues of cross-browser event handling. Let’s not concentrate on that for the moment but go for old school onevent handlers as we’re talking about different things here.

The code starts off as:

collapser = function(){
var secs = document.getElementsByTagName(‘div’);
for(var i=0;i

  • Step 1: Remove look and feel
  • Step 2: Remove obvious speed issues
  • Step 3: Removing every label and name from the functional code
  • Step 4: Use human-readable variable and method names
  • Step 5: Comment, sign and possibly eliminate the last remaining clash with other scripts
  • Which has you ending up with:

    // Collapse and expand section of the page with a certain class
    // written by Christian Heilmann, 07/01/08
    (function(){

    // Configuration, change CSS class names and labels here
    var config = {
    indicatorClass : ‘collapsible’,
    collapsedClass : ‘collapsed’,
    collapseLabel : ‘collapse’,
    expandLabel : ‘expand’
    }

    var sections = document.getElementsByTagName(‘div’);
    for(var i=0,j=sections.length;i

    Posted by Dion Almaer at 8:12 am
    6 Comments

    +++--
    3.9 rating from 17 votes

    6 Comments »

    Comments feed TrackBack URI

    Step 0: check your strings are delimited correctly…

    if(sec.style.display === none’){

    :-P

    Comment by Andy Stevens — February 8, 2008

    I really don’t understand why should something that must be clicked be a anchor with “#” href. For that we have events that handles action.
    Why not just plain span with onclick event ?

    Comment by MaLi — February 8, 2008

    You can’t do the same styling on a span as you can on a link on older browsers.

    Comment by genericallyloud — February 8, 2008

    @MaLi: I do it the same way, because links are usually already styled to look clickable ;)

    Comment by besh — February 8, 2008

    Actually it is because links and buttons are accessible by keyboard and for screen readers, spans with click events are not. If there is a thing in HTML that does the job, use this, as it has the job for a reason.

    Comment by Chris Heilmann — February 8, 2008

    And the element here should be a ‘button’ – if only styling those wasn’t such pain in the ass… But I agree that ‘a’ is much better then ‘span’ for example.

    Comment by ffreak — February 9, 2008

    Leave a comment

    You must be logged in to post a comment.