Spotting bad JavaScript tutorials

Published February 19, 2017 07:20 am | Updated December 4, 2017 08:11 am

It used to be that you could find very few truly good JavaScript tutorials online. Thankfully, the state of JavaScript documentation has greatly improved in the last few years, thanks in part to a campaign headed by Mozilla. Unfortunately there are still a lot of bad, outdated tutorials out there. A lot of people learn JS by copying code they find and tweaking it to figure out how it works. Following bad examples can teach you lots of bad habits.

If you are new to JavaScript it can be hard to spot potentially bad examples that could lead you astray. This article will discuss a number of code smells that can tip you off to information that might be outdated and/or poorly written.

Code to avoid like the plague

If code contains any of these things it is almost certainly very old and outdated or written by someone who doesn't know what they are doing. You are unlikely to find good code that follows modern best practices.

JS code wrapped in an HTML comment

Often in old code samples you'll see the JavaScript wrapped in a HTML comment:

<script>
<!--
  // some code here
//-->
</script>

When a browser encounters a tag it doesn't know about it just displays its contents; if you wrap some text in a <foo> tag, the tag will be ignored and the text will just be displayed in the flow as if it were not wrapped in a tag. Because of this, the code in any inline <script> tags would be displayed on the page in older browsers that weren't aware of it.

Wrapping the code in an HTML comment by starting it with <!-- and ending it with // --> was a hack to hide the code from old browsers. The time when this was necessary has long since passed. Any browser made after about 1996 supports JavaScript and doesn't need the content of script tags hidden. JavaScript was added to IE in Internet Explorer 3.0. Unless you expect someone using Netscape 1 or Mosaic or IE < 3 visiting your site you don't need to hide it with a HTML comment.

Code that uses the language attribute on <script> tags

The language attribute was deprecated a long time ago in HTML 4.01. The values it accepts were never standardized. Any code that uses it is very old and out of date.

Code that uses document.write

document.write has a lot of problems and should be avoided. Code that makes heavy use of it tends to be very old code that was written before the DOM existed.

For inserting things into the page, using DOM APIs is a better alternative. For displaying things for debugging purposes, console.log and its relatives are a better choice.

Code that uses arrays to build strings instead of the concatenation operator

String concatenation used to be pretty slow in JavaScript. When building large strings, it used to be a best practice to build them as an array and then concatenate them all at once using Array.prototype.join. That is no longer the case, because they knew that string concatenation was slow all of the browsers focused on speeding it up. Today simply concatenating the string with + tends to be faster.

Instead of:

var a = 'a',
  b = 'b',
  c = 'c',
  strings = [a, b, c],
  myNewString;

myNewString = strings.join('');

Just do this:

var a = 'a',
  b = 'b',
  c = 'c',
  myNewString;

myNewString = a + b + c;

Implicit globals

In JavaScript variables are declared with var (ES2015 also adds let and const). If a variable is used without being declared it becomes a global variable attached to the global object. This is bad and can lead to many frustrating bugs. It is important to always declare your variables

The with statement

The with statement was intended to make life easier by allowing you to access properties of an object within your current scope without the need to preface them with the object:

var anObjectWithAReallyLongName = {x: 1, y: 1};

with (anObjectWithAReallyLongName) {
  if (x === 1) {
    y +=1;
  }
}
// anObjectWithAReallyLongName is now {x:1, y: 2}

The problem with with is that it creates ambiguity which can make code hard (if not impossible) to read and understand. It is so troublesome it was banned in strict mode. A better approach is to just get a reference to the object with a shorter name:

var anObjectWithAReallyLongName = {x: 1, y: 1};

var o = anObjectWithAReallyLongName;
if (o.x === 1) {
  o.y +=1;
}
// anObjectWithAReallyLongName is now {x:1, y: 2}

There are a (very few) very advanced things that can only be done with with, but if you are a beginner you won't run into them.

Browser detection

It used to be that people would read a browser's user-agent string to try to figure out what browser it was. This never really worked well and has led to the indecipherable messes user-agent strings are today. Things like mozilla and like gecko were shoved into the user-agent string of non-mozilla, non-gecko browsers to trick browser detection scripts into displaying correctly in newer browsers that the scripts did not know about.

Browser detection failed rather spectacularly back when Opera 10 was released. Detection scripts naively searched the string for "Opera 1", not accounting for the fact that Opera would eventually reach version 10. This caused several large sites to block the brand new Opera 10 because it was "too old". Even worse, it is trivial to change what user-agent your browser reports via browser settings or extensions. Some proxy servers also change the user-agent string. You can't trust the user-agent string to be accurate.

A better approach is to use feature detection to see if something you want to use is available and then act appropriately instead of guessing what browser it is and doing what you think that browser supports.

Any mentions of Netscape or IE < 7

Nescape's browser was discontinued in 2008, it stopped being relevant long before that, the last several versions were little more than a rebranded version of Firefox. Any tutorial that mentions Netscape is probably more than a decade out of date.

Likewise anything that mentions IE 6 or below is bound to be ancient. Few sites still support even IE7 or 8. IE11 and Edge have been around for a while, IE9 is probably the oldest still somewhat relevant version of IE. Any modern JavaScript tutorial should be written in a fairly browser agnostic way, any browser released in the last 5 years or so (including IE9) supports most advanced JavaScript features, not to mention the kind of things you would be doing in a tutorial aimed at beginners.

Code that capitalizes non-constructor names

In JavaScript it is convention to only capitalize names that are constructor functions. If code uses capitalized names for non-constructor functions or other things it is either extremely old and written before this convention existed or written by an author who was unaware of the convention, either way there will probably be other bad habits on display in the code.

Note: When I say Capitalize, I mean words that start with a capital letter. It is normal and common to use camelCased words for variables in JS. Also, ALLCAPS variable name are used as a convention to signify a variable is (intentionally) global.

Code that uses eval

The eval function is often abused in older code written by someone who didn't know JavaScript well. There are some appropriate uses for eval, but you won't find them in a beginners tutorial. If eval is used in an example aimed at novices it probably isn't an example of something you should be doing.

One common misuse of eval was to use it to access properties of objects using a variable:

var fruit = {
  apple: 1,
  banana: 2,
  orange: 1.5
};

var chosenFruit = "banana"

var price = eval("fruit." + chosenFruit);

Code like that could be better written using bracket notation

var fruit = {
  apple: 1,
  banana: 2,
  orange: 1.5
};

var chosenFruit = "banana"

var price = fruit[chosenFruit];

This sort of misuse was most common concerning DOM elements, as Nicholas C. Zakas shows in his example.

You'll also see lots of things like:

eval("document.all." + someElementId)

This uses IE's proprietary document.all to access an element with the id that is stored in someElementId. Speaking of which...

Code that uses document.all or document.layers

Before there was a standardized DOM both Netscape and IE had their own proprietary APIs for accessing elements on the page, they were document.layers and document.all respectively.

document.layers was only used in Netscape Navigator 4 and discontinued in the next version. document.all was supported longer, it was only just dropped in IE11. Neither was ever standardized and should be avoided in favor of standard DOM methods.

Code that uses implicit evals

If you pass a string as the first parameter to setTimeout or setInterval, the code you pass is implicitly evaled. This opens you up to all the problems with eval, including the speed impact of using it.

Instead of

setTimeout("someFunction()", 1000);

Just pass the function reference and let setTimeout call it for you.

setTimeout(someFunction, 1000);

If you need to pass parameters into the function you can do that too with .bind.

Instead of

setTimeout("someFunction(1, 2, 3)", 1000);

bind creates a new function to pass to setTimeout with your parameters baked in:

setTimeout(someFunction.bind(null, 1, 2, 3), 1000);

Inline event handlers

Nowadays it is good form to use unobtrusive JavaScript that does not commingle with your HTML. Before we had addEventListener JavaScript events used to be attached as attributes to HTML element such as onClick:

<a href="#" onClick="alert('Hello');">Say hello</a>

This links to an unnamed anchor on the page (causing us to stay on the same page) and runs the code in the onClick attributes' value. A better way of doing this would be to give the link an id and then add the click handler using addEventListener:

<a id="hello">Say hello</a>
<script>
  document.getElementById('hello').addEventListener('click', function (evt) {
    alert('Hello');
  }, false);
</script>

The code is no longer tightly coupled to the element.

Even worse than using a link to an empty anchor was scripts that used the javascript protocol in the href:

<a href="javascript:void(0);" onClick="alert('Hello');">Say Hello</a>`.

javascript:void(0); literally does nothing. This was done because the onClick handler used to not fire if an anchor didn't have a href attribute. As a matter of fact you used to only be able to use onClick on links. That is no longer true, you can use click handlers on any element now:

<span id="hello" onClick="alert('Hello');">Say Hello</span>

Again, it would be better to do it unobtrusively:

<span id="hello">Say Hello</span>
<script>
  document.getElementById('hello').addEventListener('click', function (evt) {
    alert('Hello');
  }, false);
</script>

For more info about the javascript: protocol and it's intended use, read more here. More information about void(0) can be found here.

For accessibility reasons, if you are going to attach click handlers to a non-link, it is best to use a <button type="button">, instead of another type of element. Buttons are included in the tab order by default, and pressing the spacebar or enter key fires off the click handler just like a link behaves. If you use a span or other element in order to make it accessible, you would have to assign a tabindex property to the element and in addition to the click handler, you would need to add a key press handler that fires the click handler when either spacebar or enter are pressed. It is much easier to use a button, and today's browsers can style a button to look like anything you want. (Older browsers didn't used to be able to style buttons as much as other elements.)

Code or tutorials that make prominent mention of DHTML

In the late 90s, it became very fashionable to use DHTML or "Dynamic HTML". DHTML isn't anything special, it's just the same mix of JavaScript, HTML and CSS that we still use today. But the term DHTML has become antiquated and is hardly ever used anymore. If a tutorial purports to be teaching you DHTML, it is probably very outdated. Most modern tutorials would probably use a term like "web app" or "DOM manipulation" instead.

I wasn't sure if I should put this one in this section or the next "things that might be bad" section, it probably sits on the line between the two.

Code that might be bad

These aren't necessarily deal breakers, but you should probably take anything said with a grain of salt if a tutorial uses them.

<script> tag with a type="text/javascript" attribute

The type attribute was intended as a replacement for the deprecated language attribute. It isn't needed. HTML5 officially defaults the script tag to JavaScript, but even before that every browser that has supported the script tag has defaulted to JavaScript, using type has never been needed.

Also although all browsers understand text/javascript to mean JavaScript, it is technically wrong. The type attribute is supposed to contain a valid MIME type, text/javascript is not the MIME type for JavaScript, application/javascript is. It is also unneeded because if the server sends a header with a MIME type, the browser completely ignores the type attribute, and as mentioned before it defaults to JavaScript even if there is no header or type attribute. Using type just takes up unnecessary space.

For more information on the type and language attributes and why they are not needed see this speech by Douglas Crockford.

Note: In the future type="module" will be used for ES2015 modules.

Creating arrays with the Array() constructor instead of array literals

In JavaScript there are two ways of creating an array, using a constructor new Array(1, 2, 3) and using an array literal [1, 2, 3]. Array literals were not initially in JavaScript and were added later, so lots of old code still uses the constructor form. In most cases array literals should be used, they are shorter and less confusing.

The Array constructor can be confusing because it has two different function signatures new Array(element0, element1[, ...[, elementN]]) and new Array(arrayLength) that behave differently:

[1, 2, 3] // creates an array containing 1, 2 and 3
[3] // creates a single element array containing 3

new Array(1, 2, 3); // creates an array containing 1, 2 and 3
new Array(3); // creates an empty array with a .length of 3,
              // not a single element array containing 3
new Array('3'); // creates a single element array containing the string '3'

Also, literals tend to be faster.

If you are using ES2015, it adds the Array.of method. It works kind of like the Array constructor, but doesn't have the same flaw of multiple function signatures; it only accepts a list of elements to put in the new array:

Array.of(3); // returns an array containing a single number 3

ES2015 also adds the ability to spread arrays, which can be useful both with array literals and Array.of:

var abc = ['a', 'b', 'c'];

// both produce an array like ['a', 'b', 'c', 1, 2, 3]
[...abc, 1, 2, 3];
Array.of(...abc, 1, 2, 3);

Code that uses parseInt without a radix

A lot of beginners don't realize that the parseInt function takes a second parameter called radix. The radix specifies what numeric base should be used when interpreting the number (base 10 AKA decimal, base 8 AKA octal, base 16 AKA hexadecimal, base 2 AKA binary, etc).

A radix should always be used when using parseInt to ensure it is clear what base you want the number to be interpreted as, not only for maintainability reasons but also because of the unfortunate past of parseInt. As of ES5, browsers are always supposed to default to base 10 if a radix is not supplied. Previous to ES5, it was more ambiguous. Usually it would default to base 10, but if a string passed to parseInt started with a 0 it would be interpreted as octal.

So, if you were parsing a date and passed it the string '09', expecting to get a 9 back, you would instead get 0 back! As I said, this behavior was banned in ES5, and most browsers in use today support ES5. BUT some browsers have disobeyed the standard and continue to behave the old way to maintain backwards compatibility.

Things are getting better, but it is probably best to keep using a radix to ensure the code is bulletproof. It also makes the intention of your code completely clear to anyone who is reading it (including you) so the code will be easier to maintain in the future.

Again, this is something that isn't necessarily wrong, but a lot of old code probably omits the radix out of ignorance rather than understanding.

Code that relies on semicolon insertion

This one might be controversial considering the ongoing religious war over whether it is good style to use or omit semicolons in JavaScript code.

JavaScript code requires semicolons, but it also contains a feature called Automatic Semicolon Insertion which tries to guess where you should have put a semicolon and inserts one for you. This means in most cases they can be left out and the interpreter will insert them for you with no problems. If you know how ASI works, you can usually write without semicolons without problems, assuming you don't slip up and make a mistake.

There are some very good programmers who prefer to leave semicolons out, but there are also a lot of bad tutorials that leave them out out of ignorance rather than understanding how ASI works. Combined with anything on this list or anything that doesn't look quite right it is probably a red flag.

The Wikipedia article on JavaScript syntax does a pretty good job of explaining some of the issues with ASI and some of the approaches different people take to it. I personally prefer to always explicitly use semicolons because it makes the intention of your code more clear, leaves less room for error and allows me to just code without having to think about ASI at all.

Conclusion

There are probably other smells I failed to mention, if you notice any please mention them in the comments and I might integrate them into the article.

Using a linter can help you avoid the many pitfalls of these and other bad techniques. Several linters are available for JavaScript:

  • JSLint The original, very strict and opinionated. It "will hurt your feelings".
  • JSHint Less opinionated but it will still point out things that may cause you problems.
  • ESLint Extremely customizable, has a recommended set of rules you can opt into (I would recommend using them) but also allows you to pick and choose error and style rules you would like it to look for.

Always using strict mode can also go a long way to helping you avoid some of these mistakes.

Further reading

Tags:

Previous: Converting the UselessCode.org Blog to a static site

Next: Always declare your variables in JavaScript!