56

I find that what not to do is a harder lesson to learn than what should be done.

From my experience, what separates an expert from an intermediate is the ability to select from among various seemingly equivalent ways of doing the same thing.

So, when it comes to JavaScript what kinds of things should you not do and why?

I'm able to find lots of these for Java, but since JavaScript's typical context (in a browser) is very different from Java's I'm curious to see what comes out.

BobbyShaftoe
  • 28,337
  • 7
  • 52
  • 74
Allain Lalonde
  • 91,574
  • 70
  • 187
  • 238
  • 7
    What little I've seen on it, the whole language is an anti-pattern. I'm constantly amazed that Sun didn't sue over the pollution of the "Java" brand. – Paul Tomblin Dec 18 '08 at 14:43
  • 5
    @Paul: really? What's widely considered the best scripting language available? That's dynamic, extensible and has first class functions and the best closure implementation I've seen. You're trolling :/ – annakata Dec 18 '08 at 14:47
  • 1
    No, I'm not trolling. It looks messy and undisciplined to me. The only reason it's "considered the best scripting language available" is because it's the only scripting language available on every browser without installing plugins. – Paul Tomblin Dec 18 '08 at 14:51
  • Oh, and the fact that every Javascript script I've ever seen has to work around the fact that every version of IE implements it differently (and none of them get it right) isn't what I'd call a good thing. – Paul Tomblin Dec 18 '08 at 14:55
  • This is just very ill-informed. The differences between browsers is really minimal and trivially abstracted since about 2001. – annakata Dec 18 '08 at 15:01
  • What did you use in 2001 as an abstraction layer? – Greg Dean Dec 18 '08 at 15:21
  • A JS method I wrote once, same as every other sane webdev. – annakata Dec 18 '08 at 16:07
  • 1
    I wouldn't exactly call everyone writing their own abstraction layer trivial. 2001 is pushing it a little (Development of YUI began in 2005, jQuery was released in 2006) – Greg Dean Dec 18 '08 at 16:17
  • I agree with annakata here -- javascript is a serious contender for my 'favorite language' position, even given it has more serious issues (in spec and in implementation) than any other major language. – Jimmy Dec 18 '08 at 19:40
  • I'm with you, Jimmy and annakata. The concepts behind the language are simply amazing. – Matt Kantor Dec 20 '08 at 05:13
  • 9
    Highly recommend you read "Javascript: The Good Parts" for some perspective about JavaScript being "messy and undisciplined" -- the language has warts, but often enough, it's the script developers that are "messy and undisciplined", and they'd be like that in any language :) – Tony Arkles Mar 23 '09 at 16:41
  • 1
    The browser APIs != Javascript. Paul's negative comments about JS seem to be actually complaints about the browser APIs (ie their DOM APIs). As far as that is the case, I agree. Javascript is really let down by browser inconsistencies in their APIs. – thomasrutter May 04 '09 at 00:21
  • Shouldn't this thread be Community Wiki? – Steve Harrison Jul 07 '09 at 00:36
  • 1
    @annakata: the answer is Python, right? Though you left out the part about it having a standard library. – intuited Aug 21 '10 at 09:33
  • @Paul - Regarding the pollution of the 'Java' brand name, don't you realized that most official references call it EMCA Script, not Javascript. Microsoft Visual Studio calls it 'JScript' for this reason too. 'Javascript' is really the unofficial name. Soon to be called Oraclescript. – Joseph Lust Mar 31 '11 at 20:25
  • @Twisted, the official name was Javascript when it was new. ECMAScript was an unsuccessful attempt to get people to stop using the word "Java". – Paul Tomblin Mar 31 '11 at 20:30

10 Answers10

50

Language:

  • Namespace polluting by creating a large footprint of variables in the global context.

  • Binding event handlers in the form 'foo.onclick = myFunc' (inextensible, should be using attachEvent/addEventListener).

  • Using eval in almost any non-JSON context

  • Almost every use of document.write (use the DOM methods like document.createElement)

  • Prototyping against the Object object (BOOM!)

  • A small one this, but doing large numbers of string concats with '+' (creating an array and joining it is much more efficient)

  • Referring to the non-existent undefined constant

Design/Deployment:

  • (Generally) not providing noscript support.

  • Not packaging your code into a single resource

  • Putting inline (i.e. body) scripts near the top of the body (they block loading)

Ajax specific:

  • not indicating the start, end, or error of a request to the user

  • polling

  • passing and parsing XML instead of JSON or HTML (where appropriate)

Many of these were sourced from the book Learning JavaScript Design by Addy Osmati: https://www.oreilly.com/library/view/learning-javascript-design/9781449334840/ch06.html

edit: I keep thinking of more!

Stephen M Irving
  • 1,324
  • 9
  • 20
annakata
  • 74,572
  • 17
  • 113
  • 180
  • On the last point in AJAX, the X stands for XML, If you aren't passing XML, you aren't even doing AJAX, you are doing AJAJSON, or maybe just AJAJ. I'm not sure what the difference is between passwing JSON and passing XML. – Kibbee Dec 18 '08 at 14:54
  • 3
    Yeah I know what the X stands for :) Alot of the time the A is false as well. It's just an unfortunate umbrella term for stuff a lot of us were doing ages ago. FYI the difference between JSON and XML is large, but critically JSON is lighter and is in a native format JS doesn't need to parse. – annakata Dec 18 '08 at 14:58
  • AJAX is now used as an umbrella term, like DHTML. Although strictly speaking you're correct. Anything that allows for Asynchronous communication with the server is AJAX in my book. – Allain Lalonde Dec 18 '08 at 14:59
  • @annakata: Why not post each of these as a separate answer. That way, you get more points and the best single answer floats to the top? – Allain Lalonde Dec 18 '08 at 15:23
  • 4
    Kibee, I think that is just a purely pedantic distinction. This is what everybody means by the term AJAX so I think it is just irrelevant. Just forget about AJAX as an acronym and think of it as the thing we all mean. – BobbyShaftoe Dec 18 '08 at 15:26
  • 1
    @Allain: karma is irrelevant compared to putting the answer in one easy to find place - besides I was under the impression this was the preferred approach on SO (confirm anyone?) – annakata Dec 18 '08 at 16:06
  • I agree mostly with this - except the last part about XML. I think that's a flavor choice. I like my data output to come in XML because then it's easily accessible to different kinds of applications. Using a library like jQuery also makes traversing XML as simple as accessing a JSON property. – Andy Baird Dec 18 '08 at 19:06
  • @andybaird - why can't you expose JSON and XML? My apps typically work with XML internally and XSLT to JSON where required. jQuery makes it easy, but JSON still performs better there. – annakata Dec 18 '08 at 20:26
  • @annakata - I don't see the need to do both when one works just fine for both. May perform better, but it's marginal enough that I'd rather save the development time. – Andy Baird Dec 19 '08 at 00:15
  • @andybaird - we'll call it personal choice then, but imho you'll gain mileage out of a write-once JSON serialiser and the dev overhead is minimal. – annakata Dec 19 '08 at 08:37
  • 1
    What sort of 'polling' are you referring to under 'Ajax specific'? A lot of JS frameworks like jQuery do their own polling of the DOM or the XMLHttpRequest object, for instance. jQuery for example has lots of "setTimeout" calls with comments like "continually check to see if the document is ready". – thomasrutter Apr 01 '09 at 00:22
  • Can you explain to me your second point? Apart from able to attach only one event, what other drawbacks does 'foo.onclick = myFunc' have? – instantsetsuna Nov 02 '10 at 14:19
  • 1
    @instantsetsuna - That's the chief one, but the flipside is that you never know what functionality you just implicitly killed when you bind to an event directly again. It's far safer to use attach because you have to be explicit about removing it. – annakata Nov 03 '10 at 09:29
20

Besides those already mentioned...

  • Using the for..in construct to iterate over arrays
    (iterates over array methods AND indices)

  • Using Javascript inline like <body onload="doThis();">
    (inflexible and prevents multiple event listeners)

  • Using the 'Function()' constructor
    (bad for the same reasons eval() is bad)

  • Passing strings instead of functions to setTimeout or setInterval
    (also uses eval() internally)

  • Relying on implicit statements by not using semicolons
    (bad habit to pick up, and can lead to unexpected behavior)

  • Using /* .. */ to block out lines of code
    (can interfere with regex literals, e.g.: /* /.*/ */)

    <evangelism> And of course, not using Prototype ;) </evangelism>

Kenan Banks
  • 207,056
  • 34
  • 155
  • 173
14

The biggest for me is not understanding the JavaScript programming language itself.

  • Overusing object hierarchies and building very deep inheritance chains. Shallow hierarchies work fine in most cases in JS.
  • Not understanding prototype based object orientation, and instead building huge amounts of scaffolding to make JS behave like traditional OO languages.
  • Unnecessarily using OO paradigms when procedural / functional programming could be more concise and efficient.

Then there are those for the browser runtime:

  • Not using good established event patterns like event delegation or the observer pattern (pub/sub) to optimize event handling.
  • Making frequent DOM updates (like .appendChild in a loop), when the DOM nodes can be in memory and appended in one go. (HUGE performance benefit).
  • Overusing libraries for selecting nodes with complex selectors when native methods can be used (getElementById, getElementByTagName, etc.). This is becoming lesser of an issue these days, but it's worth mentioning.
  • Extending DOM objects when you expect third-party scripts to be on the same page as yours (you will end up clobbering each other's code).

And finally the deployment issues.

  • Not minifying your files.
  • Web-server configs - not gzipping your files, not caching them sensibly.

<plug> I've got some client-side optimization tips which cover some of the things I've mentioned above, and more, on my blog.</plug>

Rakesh Pai
  • 26,069
  • 3
  • 25
  • 31
9
  • browser detection (instead of testing whether the specific methods/fields you want to use exist)
  • using alert() in most cases

see also Crockford's "Javascript: The Good Parts" for various other things to avoid. (edit: warning, he's a bit strict in some of his suggestions like the use of "===" over "==" so take them with whatever grain of salt works for you)

Jason S
  • 184,598
  • 164
  • 608
  • 970
  • also I take issue with some of the things Crockford says (e.g his stance on with and continue) – annakata Dec 18 '08 at 15:10
  • the exception on browser detection is if the browser reports that it supports a method or property, but you know that the implementation is wrong/flawed. IE supports elem.setAttribute(name,value) but it certainly doesn't support it correctly. – scunliffe Dec 18 '08 at 15:23
  • @annakata: good point, i do too – Jason S Dec 18 '08 at 16:11
8

A few things right on top of my head. I'll edit this list when I think of more.

  • Don't pollute global namespace. Organize things in objects instead;
  • Don't omit 'var' for variables. That pollutes global namespace and might get you in trouble with other such scripts.
Vilx-
  • 104,512
  • 87
  • 279
  • 422
7

any use of 'with'

with (document.forms["mainForm"].elements) {
input1.value = "junk";
input2.value = "junk"; }

Greg Dean
  • 29,221
  • 14
  • 67
  • 78
  • I wonder if this will stop being an issue when we get better code analysis tools for Javascript – Allain Lalonde Dec 18 '08 at 15:22
  • 1
    See here: http://stackoverflow.com/questions/61552/are-there-legitimate-uses-for-javascripts-with-statement for a discussion of "with"; IMHO, there's at least one valid use for it, probably more... but as a means of scope control, not a convenience for modifying object members. – Shog9 Dec 18 '08 at 17:51
5

any reference to

document.all

in your code, unless it is within special code, just for IE to overcome an IE bug. (cough document.getElementById() cough)

scunliffe
  • 62,582
  • 25
  • 126
  • 161
5

Bad use of brace positioning when creating statements

You should always put a brace after the statement due to automatic semicolon insertion.

For example this:

function()
{
    return
    {
        price: 10
    }
}

differs greatly from this:

function(){
    return{
        price: 10
    }
}

Becuase in the first example, javascript will insert a semicolon for you actually leaving you with this:

function()
{
    return;    // oh dear!
    {
        price: 10
    }
}

Using setInterval for potentially long running tasks.

You should use setTimeout rather than setInterval for occasions where you need to do something repeatedly.

If you use setInterval, but the function that is executed in the timer is not finished by the time the timer next ticks, this is bad. Instead use the following pattern using setTimeout

function doThisManyTimes(){
    alert("It's happening again!");
}

(function repeat(){
    doThisManyTimes();
    setTimeout(repeat, 500);
})();

This is explained very well by Paul Irish on his 10 things I learned from the jQuery source video

Swaff
  • 13,548
  • 4
  • 26
  • 26
  • 2
    `"If you use setInterval, but the function that is executed in the timer is not finished by the time the timer next ticks, this is bad."` Why is this bad? What happens? – Kirk Woll Mar 31 '11 at 21:43
  • @Kirk That depends on what the function does, if the function alters the view, then initiates an ajax request, who's callback does something to the view, it would be highly undesirable for this sequence to appear out of sync. Stocks data for example. – Swaff Mar 31 '11 at 21:48
  • Javascript is event-driven and everything in one execution context happens in one thread in the order it was queued. There is no negative consequence of using setInterval with a function that takes a long time - it's absolutely fine. Nothing will "interrupt" other code. It will simply mean that the next call to the function will be delayed until after the current call has ended. – thomasrutter Apr 02 '14 at 23:50
  • In your example of a function initiating an ajax request - if it's a synchronous request the rest of the function will simply wait until the request finishes, then the rest of the function will run, and then any queued functions from subsequent setinterval ticks will execute after that. If it's an asynchronous AJAX call then you would have had to deal with the possibility the callback may come in an unpredictable order relative to other callbacks *anyway*, which setinterval vs settimeout doesn't change. – thomasrutter Apr 02 '14 at 23:54
4

Not using a community based framework to do repetitive tasks like DOM manipulation, event handling, etc.

Allain Lalonde
  • 91,574
  • 70
  • 187
  • 238
  • My personal choice would be jQuery, but the actual framework matters little. As long as it's not home grown. – Allain Lalonde Dec 18 '08 at 15:04
  • 6
    I sort of disagree with this - Sometimes a special solution is better than a general one and I have quite a lot of my own JS which I believe superior to jQueries equivalent. Possible I should give this stuff to jQuery actually, but the point is the masses aren't always right. – annakata Dec 18 '08 at 15:10
  • 1
    then we'll agree to disagree. :) Even if your personal framework is a better match, the testing you'd have to do, you get for "free" by using a community based approach. – Allain Lalonde Dec 18 '08 at 15:20
  • 3
    well not really, I've been dragging this chunk of code with me for 5 years now :) – annakata Dec 18 '08 at 15:21
  • 3
    Dragging is an apt metaphor. :) – Allain Lalonde Jan 12 '09 at 11:55
  • I kinda feel that the big frameworks are a bit bulky, when you want to use a small subset of things. Probably YUI has the right idea in this regard in that you can just load the modules you need, though it is very verbose for what it does. But if you chop down jQuery to size you get into a situation where it's like maintaining your own jQuery fork. I don't know what the answer is – thomasrutter May 13 '09 at 00:27
0

Effective caching is seldomly done:

  • Don't store a copy of the library (jQuery, Prototype, Dojo) on your server when you can use a shared API like Google's Libraries API to speed up page loads
  • Combine and minify all your script that you can into one
  • Use mod_expires to give all your scripts infinite cache lifetime (never redownload)
  • Version your javascript filenames so that a new update will be taken by clients without need to reload/restart (i.e. myFile_r231.js, or myFile.js?r=231)
Joseph Lust
  • 19,340
  • 7
  • 85
  • 83