RFD: How To Recognize Bad Javascript Code

[Anthony Levensalor]

Thomas 'PointedEars' Lahn posted :

If you do think that could do any good, you are using the wrong medium.
You are back out of my killfile, for now, but scored Lowest.

Could you put me back in?

~A!

 

[Jeremy J Starcher]

Care to elaborate?

The bottom section of this page:

<URL: http://bclary.com/2004/08/27/javascript-version-incompatibilities >
and
<URL:
http://books.google.com/books?id=xn...ots=FJ3VN8MMLy&sig=6AXaPChhoLEOFijvaXLV72CwIa >

(Sorry for the URL lengths.) Memory says there was one other issue
regarding arrays and Javascript1.2, but I can't recall now.

2. 'Using "href:javascript"'

[...]
There are other points that I have also mentioned in my FAQ
notes last year. There are also exceptions to be made in
special cases.

Wow. You know stuff I didn't even know I didn't know. Those are enough
of an edge-case I doubt I will include them in this article though. Its
not the sort of thing most folks will find looking for code snippets.

I know the JSON reference implementation but I don't see how that would
provide a reason for your statement that it would be easy to address
security concerns that using eval() with JSON would introduce. Care to
elaborate?

Either we are talking cross-purposes here, or I am really missing
something. It is my understanding that the JSON.parser has enough
checking to filter out anything that isn't data and therefore it "easily
addresses the security concerns that using eval()" introduces.

 

[Jeremy J Starcher]

Jeremy J Starcher said the following on 1/1/2008 2:38 PM:

Some of the first copy/paste scripts I ever used came from there. It
took a while for me to understand what was wrong with them. The first
tutorial I ever went through (web-based) was on the HTMLGoodies site
when Joe Burns was writing them. Not the best quality but at the time
the best I could find. Was enough to "Get my feet wet" and obtain the
desire to know more.

Now, I wish a good one did exist.

I know the "Code Worth Recommending Project" has taken on some of that.
They are going through some growth pains and sorting out some of the
basics. I'd be willing to donate code once they are ready for
higher-level functions.

As for a good tutorial: I've actually thought about writing one. Its
been over a decade since I've done any real technical writing, but I think
I could find the touch again. Like all of us, the trouble is time. With
all the crap on the market, I don't know if my book would actually -sell-
and I'd feel obligated to c.l.j for at least -part- of the money I made on
the second edition, because the first edition will be red-lettered to
death from you guys. *grin* Not a bad thing, but I can't easily post a
credit-card number here and say 'Withdraw what you earned.'

 

[Jeremy J Starcher]

On Tue, 01 Jan 2008 13:38:43 -0500, Randy Webb wrote:

[regarding javascript: protocol]

The group FAQ, and the Notes pages on it, cover just about every aspect
of javascript: protocols.

I'll re-read the FAQ again, thanks.

try/catch is another of the things you didn't cover. Don't use it.

I've not used try/catch in Javascript myself, but I've done reading on it.

According to <URL: http://www.w3schools.com/js/js_try_catch.asp >
try...catch statement is available in IE5+, Mozilla 1.0, and
Netscape 6. Googling says Opera and KHTML both have it.

I don't understand the concern ... are there common browsers out there
that don't support it? I've heard rumors about mobile devices, but
nothing concrete.

(e-mail address removed)

would seem to dictate that is the case.

 

[David Mark]

Jeremy J Starcher said the following on 1/1/2008 5:37 AM:




HTML Comments.
  Will cause problems when XHTML is used.

Script tag usage.
  text/javascript is an obsolete (although valid) MIME type.

But you have to use it for now if you want valid markup.

href:javascript.
  Drop #1, they all fall into the "Too stupid to know better" category.

document.write.
  Avoid it. Period.

For the most part. There are times when it comes in handy. I have
recently been working on test page for some proposed Code Worth
Recommending and I didn't want the HTML version to break in ancient
browsers that don't feature createElement. Of course, there is no
choice when using XHTML.

  Tables produced with document.write are indicative of an idiot programmer.

Yes, it is hard to imagine a case where tables should be generated
with document.write.

[snip]

  As for minification, that is an indication of bad coding practices.

I don't follow you there. The project I am currently working on
builds a library from various modules. If all are included, the file
size with comments and white-space is almost 200K (and will get bigger
in the future as more comments are added.) Running it through the YUI
"compressor" (minifier/obfuscator) reduces it to 90K. Granted, this
should only be done when the code is deployed on a production server.
One irritation is that I have to re-test everything to make sure that
minification didn't break something, but I have yet to have a problem
with this particular minifier. Of course, I run the source through
JSLint to catch typos and missing semi-colons beforehand.

Use of eval.
  The use of eval itself usually indicates bad coding, but not always.
  Whether it is a bad use of it or not depends on how it is used.
  And a beginner can't possibly know.

Is this a good use of eval?
function convertToDecimal(fraction){
return eval(fraction)

}

I don't like it.

Where fraction is a fraction that you need converted to decimal? It can
be written like this:

function convertToDecimal(fraction){
var numerator = fraction.substring(0,fraction.lastIndexOf('/'))
var denominator = fraction.substring(fraction.lastIndexOf('/')+1)
return (numerator/denominator)

}

Testing shows that eval is a lot quicker but a newbe could never know
that it was actually a "good use" of eval.

The second version could be made more efficient. I don't know whether
it could approach the speed of using eval, but I would still prefer
it.

Browser detection.
  Spelling error with interfer versus interfere.

DOCTYPE.
  No version of IE, not just IE7, handles it that way.

JSLint.
  JSLint is a tool that attempts to make sure you code
  according to the preference and style that Douglas Crockford
  prefers.

You can filter out the stylistic concerns. Certainly I don't use the
strict whitespace option, but overall, I find it an invaluable tool.
All it takes is one mistaken keystroke to introduce an unintended
global variable (of course, Firebug spots those in its DOM view.)
Then there are unused variables, out of scope references, unfiltered
for-in loops, etc. I think it is a must for any project with more
than a few dozen lines of code.

Things you didn't cover:

The use of the "with" operator.

Always a bad idea.

Use of "new Function".

Quoting from JSLint: "Function constructor is eval." Like eval, it
occasionally has a reasonable use, but shouldn't be used as a crutch.

 

[David Mark]

On Tue, 01 Jan 2008 13:38:43 -0500, Randy Webb wrote:
[snip]

try/catch is another of the things you didn't cover. Don't use it.

I've not used try/catch in Javascript myself, but I've done reading on it.

According to <URL:http://www.w3schools.com/js/js_try_catch.asp>
try...catch statement is available in IE5+, Mozilla 1.0, and
Netscape 6.  Googling says Opera and KHTML both have it.

True enough.

I don't understand the concern ...  are there common browsers out there
that don't support it?  I've heard rumors about mobile devices, but
nothing concrete.  

Certainly there are mobile devices (and other agents) using script
engines that do not parse try-catch clauses. A good rule of thumb is
to relegate the use of try-catch to scripts that you can afford to
lose in such browsers. In other words, if you have enhancements that
will work in virtually any browser (e.g. form validation), don't lump
them in with or make them reliant upon scripts that feature try-catch
clauses (e.g. Ajax, Flash.) That way agents that ignore the scripts
with try-catch clauses can still make use of the other enhancements.

 

[Dr J R Stockton]

In comp.lang.javascript message <[email protected]
am.me.not.com>, Tue, 1 Jan 2008 05:37:14, Jeremy J Starcher

And it would really help if I included

<URL: http://www.mopedepot.com/jjs/HowToRecognizeBadJavascriptCode.html >

The encoding of the page seems undefined, causing the W3C Markup
Validator to emit /inter alia/ a split infinitive. Try Opera Ctrl-Alt-V
or otherwise. I think that
<META HTTP-EQUIV="Content-Type" CONTENT="text/html; charset=ISO-8859-1">
or similar will deal with the matter, if you cannot do better with
server settings.

The page is unusually good in that it does not overrule my choice of
font face and size; but it is bad in using boxes of light text on dark
background. Use black-on-light, discriminating by hue-of-light and/or
colour of border.

"Rational" should be "Rationale".

"When I first started to learn Javascript, I turned to Google for
examples and code snippets." - that's an almost completely reliable way
of getting bad code.

Another good sign of bad code is any sign of an automated code
generator, unless specifically known to be good.

Code within a proportional sentence needs to be monospace - e.g.
" things, Document.write's limitations " to be
" things, <code>document.write</code>'s limitations " (small d, too).

Text should not abut a colour-change - leave padding.

"import compability code" - spelling.

The page does not indicate any way of contacting the author; that
inhibits feedback.

The page lacks date or version number (apart from copyright 2008);
without those, the re-visitor cannot see whether re-reading may be
worthwhile.

The page lacks any link to a site home page or similar.

The page lacks a link to this newsgroup and its FAQ.

You have "All variables should be declared before their use." In
Pascal, "before" implies "higher up the page"; in Javascript, it may
mean "earlier in time". Clarify. In addition, the scope of variables
should be local wherever reasonable. ISTM that you mean, but do not
really say, that.

I don't like the idea of liberating the chlorine from one or two grains
of salt - when being clever, one needs also to be right.

"Permission granted to reproduce in any form ..." - Unwise. Copies
that are not updated are a disservice to their readers. Google for
exactly "other critical date lists at Cinderella" to see what I mean.

" with a ending with " - should be "an"

"from an an online "

"The eval statement allowes any "

"interfer with other browsers"

"to use use a valid "


There are many other indicators of bad code, some substantially
language-independent.

One is superfluous recalculation, as in
document.getElementById("cat").style.font = "Tahoma"
document.getElementById("cat").style.size = "88%"
where of course the cat only needs to be found once -
puss = document.getElementById("cat") ; ...

Another is (multi-)duplication of code with slight modification, as is
often seen in validation of inputs to be non-empty. Use instead a
function with parameters.

A JS-specific one is date object duplication by
D2 = new Date(D1) ; .

Another JS-specific one is conversion of String to Number by any means
other than unary + in a case where unary + serves.

IMHO, the type returned by a function should not vary unexpectedly with
input value or type. To convert a positive integer Number to a string
of at least 2 digits, function DD(J) { return J>9 ? J : "0" + J } ;
may return String or Number. Generally that won't matter, especially in
test. Sometimes it will matter.

It's a good idea to read the newsgroup c.l.j and its FAQ. See below.

 

[RobG]

And it would really help if I included

<URL:http://www.mopedepot.com/jjs/HowToRecognizeBadJavascriptCode.html>

Despite the comments here, the page does not seem to have been
updated. I'll stick to things others haven't mentioned:

HTML comments
The heading should be "HTML comments inside script elements".

Depreciated script tag usage
In the early days of Javascript and HTML the <script> tag took a
"language" parameter. With HTML 4, the language parameter became
depreciated.

That statement should be something like: "The language attribute was
introduced in HTML 4.0, it was immediately deprecated in favour of the
type attribute."

<URL: http://www.w3.org/TR/WD-html40-970708/interact/scripts.html#h-9.2.1.1
Using "href:javascript"
Should note that using script for the value of the href attribute
probably indicates that a button should be used, not a link.

I don't think the term "cargo-cult programming" applies specifically
to that section, it is more a general comment about people who copy
code or patterns and could be applied to the entire article. Having
said that, I wouldn't use it at all.

Not declaring local variables (no var)
...causes large and hard-to-find complications.

While the thrust of the section is good, it gives the impression that
using var stops global variables. Better to explain that:

1. All variables should be declared
2. Global variables should be kept to a minimum
3. Declaring variables inside functions keeps them local
4. Not declaring them inside functions makes them global
*when the function is first called*.

Also, unintentional global variables *may* cause hard to find bugs,
there is no guarantee.

Not ending lines in a semi-colon ";"

This is a contentious area. Semi-colons are *required* by the
ECMAScript specification (Section 7.9), however automatic semi-colon
insertion is prescribed as a convenience. I think that they should
always be manually inserted by the programmer, others disagree. It is
at least good style in a tutorial to manually insert semi-colons to
show where they should be.

There is also argument over the use of minifiers. I think they are a
valid way to reduce code bulk, however zipping code files is probably
as effective and doesn't require 2 sets of testing.

Web pages that do not include a DOCTYPE and/or do not validate

Whenever you are tempted to use "and/or", don't. One or the other
should be used. In any case, the title doesn't make sense. A DOCTYPE
is required for valid HTML, therefore if there is no DOCTYPE, the page
is not valid HTML.

I think the point you are trying to make is something like:

Beware of invalid markup

Primary causes of invalid markup are: missing DOCTYPE declaration, use
of invalid or deprecated tags or attributes, incorrect nesting of
elements and missing required or incorrect use of element attributes
(e.g. non-unique ID attribute values). All of the above may affect
how scripts interact with the DOM and are signs of poor quality
markup. It is likely that anyone publishing poor quality HTML applies
the same level of quality control to their scripting.

Last of all, remember that it is your document. Ensure readers will
not be confused between what is required by standards, what "works"
and what doesn't and what is good practice (i.e. opinion).

 

[RobG]

But you despise the people that made them?  Something doesn't add up here.

Yes, it does, you're just playing cute. It's not what you say, it's
the way that you say it. You've been told that /ad nauseam/.

 

[David Mark]

There is also argument over the use of minifiers. I think they are a
valid way to reduce code bulk, however zipping code files is probably
as effective and doesn't require 2 sets of testing.

GZIP compression should be handled by the Web server as it needs to
negotiate this with each client. Manually compressing files is a bad
idea as some clients will choke on them. Another reason to minify is
that compression (when available) will only reduce comment bulk,
whereas minification eliminates it.

 

[Eric B. Bednarz]

Anthony Levensalor wrote:

“‘script’ element _type_â€

The terms you were looking for are ‘start-tag’ and ‘end-tag’. See ISO
8879, clause 7.3, or otherwise not the FAQ, part 5 (and also part 4, now
that I come to think about it .

That’s but a for dummies introduction to SGML, found in a technical
“recommendation†that itself has a hard time to get all this SGML arcane
right.

Maybe you are an idiot. (See, *now* I said it.)

And he didn’t even say that there is no notion of a ‘`script'’
element type *anywhere* in HTML 4.

 

[Thomas 'PointedEars' Lahn]

“‘script’ element _type_”

`script' is definitely an element type, but "Deprecated `script' element
type usage" would indicate that the element type itself is deprecated, which
it is not.

The terms you were looking for are ‘start-tag’ and ‘end-tag’. See ISO
8879, clause 7.3, or otherwise not the FAQ, part 5 (and also part 4, now
that I come to think about it .

There is no requirement for a hyphen in compound words in English.

You are merely splitting hairs while I was pointing out the use of wrong
technical terms -- a slight, but important difference.


PointedEars

 

[Jeremy J Starcher]

The encoding of the page seems undefined, causing the W3C Markup
Validator to emit /inter alia/ a split infinitive. Try Opera Ctrl-Alt-V
or otherwise. I think that
<META HTTP-EQUIV="Content-Type" CONTENT="text/html; charset=ISO-8859-1">
or similar will deal with the matter, if you cannot do better with
server settings.

Alas, can't touch my server settings. Last time I ran the page through
the validator, it didn't complain. Curious, I'll look again. If I have
to I'll drop the META tag in.

The page is unusually good in that it does not overrule my choice of
font face and size; but it is bad in using boxes of light text on dark
background. Use black-on-light, discriminating by hue-of-light and/or
colour of border.

Semi-valid point. God bless CSS. Easy to change.

"When I first started to learn Javascript, I turned to Google for
examples and code snippets." - that's an almost completely reliable way
of getting bad code.

Agreed. However, finding ways to get GOOD code isn't easy.

Another good sign of bad code is any sign of an automated code
generator, unless specifically known to be good.

True, but difficult to explain to a beginner.

Code within a proportional sentence needs to be monospace - e.g.
" things, Document.write's limitations " to be
" things, <code>document.write</code>'s limitations " (small d, too).

Stylewise, I know your right. Wish you weren't.

Text should not abut a colour-change - leave padding.

"import compability code" - spelling.

The page does not indicate any way of contacting the author; that
inhibits feedback.

For the first draft of this document, that was fine with me.

The page lacks date or version number (apart from copyright 2008);
without those, the re-visitor cannot see whether re-reading may be
worthwhile.

Valid point.

The page lacks any link to a site home page or similar.

By design. I don't have a personal home page nor do I have a computer
related page anywhere. The page is hosted on a work server with
permission.

The page lacks a link to this newsgroup and its FAQ.

True. I am working on a "links worth having" section.

You have "All variables should be declared before their use." In
Pascal, "before" implies "higher up the page"; in Javascript, it may
mean "earlier in time". Clarify. In addition, the scope of variables
should be local wherever reasonable. ISTM that you mean, but do not
really say, that.

I'll will look at that phrasing again. Since I was referring to local
function variables, both meanings would apply.

I don't like the idea of liberating the chlorine from one or two grains
of salt - when being clever, one needs also to be right.

With a "rule of thumb" it is much harder to define "right." In practice
I strongly discourage use of document.write. In practice, I use it
sparingly.

"Permission granted to reproduce in any form ..." - Unwise. Copies
that are not updated are a disservice to their readers. Google for
exactly "other critical date lists at Cinderella" to see what I mean.

I will Google for that later. Do you have a licence template that you
recommend?

There are many other indicators of bad code, some substantially
language-independent.

Agreed again.

Your other coding points, while valid, fall outside the scope I am trying
to cover. I am trying to give readers some "rules of thumb" they can use
to avoid the worst of the scripts. Sometimes it really does boil down to
choosing the best of the worst.

It's a good idea to read the newsgroup c.l.j and its FAQ. See below.

I have. My comments were in regard to RE-READING the FAQ. I've read so
much stuff lately that things have gotten jumbled.

 

[Jeremy J Starcher]

Despite the comments here, the page does not seem to have been updated.
I'll stick to things others haven't mentioned:

Give me a break. *grin*
There have only been about nine hours since I woke up and read the first
comments and now. That time has been used in this discussion and
rewriting the page. I'll really try to get the updated version up tonight
though.

HTML comments
The heading should be "HTML comments inside script elements".

Depreciated script tag usage
In the early days of Javascript and HTML the <script> tag took a
"language" parameter. With HTML 4, the language parameter became
depreciated.

That statement should be something like: "The language attribute was
introduced in HTML 4.0, it was immediately deprecated in favour of the
type attribute."

OH... I like that phrasing.

<URL:
http://www.w3.org/TR/WD-html40-970708/interact/scripts.html#h-9.2.1.1
Using "href:javascript"
Should note that using script for the value of the href attribute
probably indicates that a button should be used, not a link.

I don't think the term "cargo-cult programming" applies specifically to
that section, it is more a general comment about people who copy code or
patterns and could be applied to the entire article. Having said that,
I wouldn't use it at all.

Not declaring local variables (no var) ...causes large and hard-to-find
complications.

While the thrust of the section is good, it gives the impression that
using var stops global variables. Better to explain that:

1. All variables should be declared
2. Global variables should be kept to a minimum 3. Declaring variables
inside functions keeps them local 4. Not declaring them inside
functions makes them global
*when the function is first called*.

Also, unintentional global variables *may* cause hard to find bugs,
there is no guarantee.

That is one of the sections I'm re-writing, trying to make it much more
clear yet ... able to grasp the concept. Some of that is more detail than
I need for this purpose.

Not ending lines in a semi-colon ";"

Yea.. that whole section is going.

Web pages that do not include a DOCTYPE and/or do not validate

Whenever you are tempted to use "and/or", don't. One or the other
should be used. In any case, the title doesn't make sense. A DOCTYPE
is required for valid HTML, therefore if there is no DOCTYPE, the page
is not valid HTML.

I think the point you are trying to make is something like:

Beware of invalid markup

Primary causes of invalid markup are: missing DOCTYPE declaration, use
of invalid or deprecated tags or attributes, incorrect nesting of
elements and missing required or incorrect use of element attributes
(e.g. non-unique ID attribute values). All of the above may affect how
scripts interact with the DOM and are signs of poor quality markup. It
is likely that anyone publishing poor quality HTML applies the same
level of quality control to their scripting.

*nod* I'll go for that. Nicely worded.

Last of all, remember that it is your document. Ensure readers will not
be confused between what is required by standards, what "works" and what
doesn't and what is good practice (i.e. opinion).

Part of the reason I asked for comments. Sometimes I'm too close to it to
read it as an outsider.

 

[John W. Kennedy]

a. The word you were looking for is _deprecated_, not "depreciated".

Actually, "depreciated" is the correct word. Some moron about 75 years
ago decided that "deprecated" sounded more kewl, and got the two words
hopelessly confused. Properly, to deprecate something is to pray to be
protected from it, ee.g.:

From all evyll and myschief, from synne, from the craftes and assaultes
of the devill, from thy wrath, and from everlastyng damnacion.

Good lorde deliver us.

From blindnes of hearte, from pryde, vaynglory, and hypocrisy, from
envy, harred and malice, and all uncharytablenes:

Good lorde deliver us.

From all fornycacion and all deadly synne, and from all the deceiptes
of the worlde, the fleshe, and the devill:

Good lorde deliver us.

From lightnyng and tempast, from plage, pestilence and famyne, from
battayle and murder, & from sodaine death:

Good lorde deliver us.

From all sedycion and privey conspiracie, from the tyranny of the
bisshop of Rome and all his detestable enormyties, from all false
doctrine and heresye, from hardnes of hearte, and conmtempte of thy
worde and commaundemente:

Good lorde deliver us.

By the mystery of thy holy incarnacion, by thy holye nativyte and
cirumcysyon, by the baptyisme, fastynge and temptacyon:

Good lorde deliver us.

By thyne agony and bluddy sweate, by thy crosse and passion, by thy
precious death and buryal, by thy glorious resurrectyon and ascension,
by the commyng of the holy Ghost:

Good lorde delyver us.

In al time of our tribulacion, in al tyme of our wealth, in the hour of
death, in the day of judgment:

Good lorde deliver us.

--
John W. Kennedy
"Those in the seat of power oft forget their failings and seek only the
obeisance of others! Thus is bad government born! Hold in your heart
that you and the people are one, human beings all, and good government
shall arise of its own accord! Such is the path of virtue!"
-- Kazuo Koike. "Lone Wolf and Cub: Thirteen Strings" (tr. Dana Lewis)

 

[Doug Miller]

Actually, "depreciated" is the correct word. Some moron about 75 years
ago decided that "deprecated" sounded more kewl, and got the two words
hopelessly confused. Properly, to deprecate something is to pray to be
protected from it, ee.g.:

Wrong on all counts. You need to find yourself a better dictionary.
"Deprecate" means to belittle; "depreciate" means to reduce the value of. The
Latin *root* of "deprecate" means to pray to be protected from, but that is
not its English meaning, properly or otherwise.

 

[John W. Kennedy]

Wrong on all counts. You need to find yourself a better dictionary.
"Deprecate" means to belittle; "depreciate" means to reduce the value of. The
Latin *root* of "deprecate" means to pray to be protected from, but that is
not its English meaning, properly or otherwise.

That /is/ its English meaning -- its only English meaning until the
early 20th century when, as I said, some moron got the two words mixed
up. The fact that modern dictionaries perforce have to acknowledge the
mistake doesn't change that.

 

[David Mark]

David Mark said the following on 1/1/2008 5:10 PM:







I do?

<script type="">
alert('Just checking')
</script>

The W3 validator validates it just fine. And all the PC based browsers I
tested it on happily gave me the alert.

So much for "valid HTML", eh?

That is valid markup (but an odd choice) and as there is type
indicated, browsers use the default of JavaScript. This is valid too:

<script type="application/javascript">
...
</script>

But is not recommended as it could confuse older browsers. So you are
left with two choices ("text/javascript" or ""), only one of which is
clear.

 

[David Mark]

David Mark said the following on 1/1/2008 10:34 PM:









As for "validity", this is valid HTML:

<script type="HikkScript">

Right. The validator only cares about the structure of the markup.

 

[Anthony Levensalor]

David Mark posted :

[snip]

Certainly there are mobile devices (and other agents) using script
engines that do not parse try-catch clauses. A good rule of thumb is
to relegate the use of try-catch to scripts that you can afford to
lose in such browsers. In other words, if you have enhancements that
will work in virtually any browser (e.g. form validation), don't lump
them in with or make them reliant upon scripts that feature try-catch
clauses (e.g. Ajax, Flash.) That way agents that ignore the scripts
with try-catch clauses can still make use of the other enhancements.

This is an excellent tip, thanks! I've got my current XMLHttpRequest
objects instantiating in try-catch blocks. (I found an article on the
net loong before I found the group)

~A!