UserJS.org

Clear and readable code

Written on 2005-06-18 14:29 by tarquin. Last modified 2005-09-19 09:19

When writing a Web page, the general readability of your code does not matter too much. The only people it really matters to are you, and anyone who will be debugging your code (including someone writing User JavaScripts to try to fix or enhance it).

With shared User JavaScripts, clarity is very important, as other people will need to ensure your code is going to do what they want it to do. With a public archive such as UserJS.org, if your code is not clear enough, it will almost certainly be rejected.

Indenting and { curly braces }

Properly indenting code is a very important part of making it readable. Function code, for and while loops, and if statements should all be indented (unless their contents are so short as to make it unnecessary). Leaving large numbers of blank lines around the curly braces does not help to improve the readability. Putting too many statements on one line also makes it much harder to read, and so does not leaving spaces around the tokens in conditionals, assignments, or function calls.

Curly braces should generally not be omitted, as they help to show which lines will be affected by a particular statement. Additionally, if anyone else needs to debug your code, adding in extra alert or postError calls inside the conditional or loop would require them to put in all of the curly braces that you left out. It is much more easy to just put them there in the first place.

This is an example of very badly formatted code:

  function foo(a,b,c,d)

{

if(a&&b&&c)

{

bar();document.body.style.color='green';if(a==b&&d>=4)for(x=1;x<d;x++){document.getElementById('item'+x).style.color='red';c++;}

}

return c;

}

This example would have been much more easy to read if it were written like this:

function foo( a, b, c, d ) {
  if( a && b && c ) {
    bar();
    document.body.style.color = 'green';
    if( a == b && d >= 4 ) {
      for( var x = 1; x < d; x++ ) {
        document.getElementById('item'+x).style.color = 'red';
        c++;
      }
    }
  }
  return c;
}

Comments

Comments are always a useful way of telling other people, and yourself, what your code is trying to do. It is not necessary to comment every single line of code, this is actually counter productive, and makes it harder to read. Generally it helps to put a comment before each section of code explaining what that piece of code does, and before any important lines whose purpose may not be immediately obvious (such as a workaround for a rendering bug).

Something simple like get a reference to each div and hide it is a good comment (assuming that is what the code actually does, of course).

For a bug workaround, it helps to say that it is a bug workaround, for example:

<span class="js-comment">//bug - force reflow to remove border lines</span>
document.body.className = document.body.className;

Descriptive variables

It is very common to see scripts whose variable names have been deliberately uglified so there is less chance of them conflicting with the code on the page. Unfortunately it also means there is less chance of anyone understanding or remembering what each variable actually represents.

For example; a function called jp104 that accepts parameters called pp931 and pp932, and uses pp931 to call the function jp103, which in turn accepts a parameter called pp547, references an object called d878 and returns its child that is called whatever value is in pp547 - jp104 then stores the return value from jp103 in a variable called v917, and then references its className, and sets it to the value of pp932. Phew!

Now, imagine it was like this; a function called changeClass that accepts parameters called divID and newClass, and uses divID to call the function getElementCalled, which in turn accepts a parameter called elementID, references an object called theDocument and returns its child that is called whatever value is in elementID - changeClass then stores the return value from getElementCalled in a variable called theDiv, and then references its className, and sets it to the value of newClass.

As you might imagine, the second one makes it much more easy to understand what is actually happening. If conflicts need to be avoided, there are much better ways. See our article on avoiding conflicts.

Too many functions

If you have ever tried to debug someone else's menu script, then you will probably be familiar with this problem. Some scripters seem to be obsessed with writing functions. Every single line of code seems to need its own function, and the scripts can consist entirely of function call after function call, many calls to functions whose sole purpose is to call another function, that in turn calls another function, that calls yet another function.

This is not a good way to write. It makes it nearly impossible to follow what your code is doing. Many things do belong in functions, such as a set of assignments and commands that you need to run repeatedly, or after an event or timeout, but going overboard can make your code unreadable.

If a function contains only one or two lines of code, or just calls to other functions, or is only ever going to be called once, then you should consider taking it out of a function and putting it back into the code that would have called it. This will make the code more easy to read, and in addition, will make it run faster, as the browser does not have to evaluate so many function calls.