For me there was a time that can only be described as adolescence in the field of programming and more specifically JavaScript. This period was characterised by a certain laziness and hubris. I thought I was right. I thought others were wrong. I could see no path but the very strict one that I had boxed myself into through a process in which I longed for certainty and the expulsion of realistic doubt.
Today I present a short list of JavaScript practices that once seemed right but I now deem foolish (in most situations):
Using logical operators as statements
a() || b.foo.bar(); m && c() ? d() : l || m.foo[bee](3); |
There is no real advantage in forgoing convention for the sake of cleverness.
Always declaring variables at the top of their scope
function calculate() { var a, m, t, y, This, is, really, understandable = false; // Actual logic goes here } |
Instead I declare a variable where it makes sense. Much of the time this is at the top of the scope, but there are enough exceptions to make me shake my head in dissapproval at the thought of a style guide imposing religious top-of-scope declarations.
Repeatedly used inline object literals
function prepareRequest(mode, updateId, value) { var req = { mode: mode, updateId: updateId, value: value, uid: genUID() }; // etc. } |
It’s better for clarity and performance (potentially) to define separate classes for cases where you repeatedly make object literals of the same structure. For example the above could be refactored into a Request class.
Complex inline regular expressions
if (/r[ -~]?(?:md+|d+[0-3]{2,})_m?$/.test(thing)) { // ... wat } |
It makes sense to cache these things. Plus naming it means someone might have a chance of understanding WTF the code does.
Using single letter variable names
They make no sense unless used in the various idiomatic loops.
Strict equality everywhere
if (typeof x === 'string' && x === 'mode:45') { // ... } |
Sometimes it’s overkill. Regular equality can be used in both of the above cases.
Assuming truthiness equals presence
if (!cachedReturnValues[key]) { cachedReturnValues[key] = value; } |
It’s potentially dangerous. E.g. It’s better to use the ‘in’ operator to discover presence, or even ‘hasOwnProperty’.
Commenting nothing or commenting every little thing
// This is a loop for (var i = 0; i < arr.length; i++) { // This is me adding one to the item in the array at the index // specified by the variable `i` declared slightly above this comment: arr[i] += 1; } // this is the end of the loop |
Commenting everything demonstrates either a lacking clarity in your code or a lacking clarity in your mind as to the intention of comments. Commenting nothing shows hubris and laziness.
Thinking I can write something reliable without unit tests
Unit tests give me the confidence to be sure that my code does what I intend it to do.
Using long names and being overly specific
A long name suggests bad API design. Re-think where you’re defining the variable/method. Think of a circumstance in which the variable name could be shorter — a context that makes sense for that variable. Then create that context. You’ll probably end up with a cleaner internal API. E.g.
// Version 1: var responseCache = {}; function generateNextIDForANewResponseCacheItem() {...} function deleteResponseCacheItemsThatDoNotEqual(value) {...} // Version 2: function ResponseCache() { this.data = {}; } ResponseCache.prototype = { genNextID: function() {...}, remove: function(optionalFnCheck) {...} }; |
Strict coherence to a standard
This never works and never solves what you think it will. It’s better to dynamically sway and constantly rethink your idea of what is right and wrong given each situation as it arises. This is why I try to steer clear of over-zealous coding style dogma, or all dogma for that matter.
Thanks for reading! Please share your thoughts with me on Twitter. Have a great day!
Besides the gotcha that variable declarations (but not initializations) are hoisted within function scope, which is sometimes a big trap for new devs, the main reason I put all variables at the top of my function scope is because of the coming `let` operator. There’s two ways to think about how `let` will affect future code:
1. Once the `let` operator is available, people will start going through and replacing `var` with `let`, probably because someone will make a jsperf test showing it’s faster. Global find-and-replace of `var` into `let` will be widespread. And what will the result be?
`var` statements improperly inside of blocks, which currently don’t act scoped to the block, will now be `let` statements inside of the blocks, and will “magically” start being scoped, which will introduce subtle harder-to-find bugs of `undefined` variables.
No such danger can occur if you currently only ever use `var` in a function scope location. The switch to `let` will always be safe.
2. If you’re in the habit of strewing `var` throughout your code, but you’re not going to do the find-and-replace madness from #1, you still have some poor patterns of code style that are likely to be introduced, where you’ll have `var` and `let` statements mixed in throughout the code, making it MUCH harder to understand and visually trace which variables exist at which points in your code.
I think a cleaner style that accounts for both `var` and future `let` usage is to have all `var` declarations at the top of functions, which is where they actually are interpreted anyway, reducing possible confusion, and to use contextual `let` declarations in places where you specifically intend a variable to only be used in some specific block scope.
In that way, you’re using `var` and `let` in a stylistic way that also matches their behavior.
Examples of my above assertions: https://gist.github.com/4062989
This was inspiring. Thank you.
Kyle: arguing that variables go on top because it allows you search-replace it with let in some distant future is kinda weak. 1) search-replace changes like that typically need manual check up anyway, 2) a good suite of tests will discover issues _if_ you go down this route, and 3) there’s little benefit to such a sweeping change.
Regarding regular expressions, I agree that naming them is good. But if I’m not mistaken, you don’t need to worry about caching – regular expression literals are singletons, e.g two equal literals will refer to the same instance.
Interesting. Is that true for all ECMAScript engines?
Long names are the path you travel to good design, because as soon as you have three methods that start with “createRequestAndFire” you know how to change your design and create a class. Getting there is quite handy with long names, because you rarely know before hand which path your design takes. #2cents
Mathias: Nope, it’s not the case in V8 or latest Firefox as far as I can tell. So I’ll redact that. It think it was the case earlier though… Sorry for the misinformation.
@Christian- you apparently are not aware of the strong and growing “let is the new var” contingent. This part of my argument is against that anti-propaganda.
I’m not saying `let` is bad or shouldn’t be used, I’m saying more needs to be done to educate the discourse on `let` so that coding style around it will not create more complexity than it’s worth.
More examples of potential issues with `let` code if coding style is not payed attention to closely:
https://gist.github.com/4065916
and
https://gist.github.com/c548116927eb896f1276
Kyle isn’t just arguing for search-and-replace functionality – you missed the part about code readability. Easily understanding code later on down the road is just as important as anything else.
Don’t forget faux classes!
James: Excellent post, as always. I remember going through the same period of adolescence—”my style is objectively correct and yours is not”—before realizing how subjective my beliefs truly were.
Mathias:
It was true for ES 3 (“Evaluation of the literal produces a reference to that object; it does not create a new object”), but not ES 5. I believe older versions of Firefox and Rhino were the only ones that followed ES 3; all others followed the de facto ES 5 behavior before it was standardized.
There’s an interesting pattern that exploits the ES 3 behavior.
@Christian: Ah, I misunderstood. I thought you were talking about how ECMAScript engines handle regex literals internally (i.e. internal engine optimizations that aren’t exposed to the JS dev).
Agree with most. My favorite:
The whole point of always using strict equality is so that you don’t accidentally misuse it. Yes, you don’t need it in your example but the minute you start letting yourself use == you’ve opened a can of worms. Next time you should use === you may not catch yourself and blame, you’ve shot yourself in the foot. The safer bet is to just use === everywhere.
@Kyle
`var` statements improperly inside of blocks, which currently don’t act scoped to the block, will now be `let` statements inside of the blocks, and will “magically” start being scoped, which will introduce subtle harder-to-find bugs of `undefined` variables.
I consider this an extremely weak argument.
Code that breaks because of the replacement of var with let, is code already broken logically.
Why was the coder using a variable outside of a block that he declared it in the first place? I can’t think of any reason, besides bad design.
That hoisting let these programmers get away with that is beside the point. Code like that should be considered broken in the first place.
The only correct practice would be to either put their variables on top, or put them in a block and constrain their usage there. In which case, var -> let substitution would work just fine.
Plus, something like JSLint would help there.
Learn so much from you, thank you.
Here are some conventions I WILL NEVER GIVE UP and are used across ALL languages in which I program:
All variable names include a scope character and a type character. For thirty years nothing which has altered has made these unnecessary in any language although of course other conventions could be used. These conventions have simplified my coding.
Scopes:
======
l for local
a for argument to a function
i for inpage
_ for global
types (even for strongly typed languages):
======
s for string
i for integer
d or dbl for double
date for date
img for image
arr for array
cl for classes
examples:
=========
ls_form, as_type, _s_type
li_index, ad_coord, _date_today
function clPerson(as_firstname, as_lastname, ai_age)
{
this.firstname = as_firstname;
this.lastname = as_lastname;
this.fullname = as_firstname + ” ” + as_lastname
this.age = ai_age;
}
var larr_people;
larr_people[larr_people.length] = new clPerson(“Mackie”, “Knife”, 43);
I really enjoy these kinds of argumentations and arguments where reading the comments is as interesting as reading the original post. I’m still adolescent enough to cling to at least half of your points as ways I like to do things. Especially strict equality checking and manually hoisting my var declarations. But the point about using long descriptive names, which is something I do and prefer over inline comments, is interesting. If you can display equal level of clarity through context then that would most likely be simpler. I’ll be experimenting with that.
Yes this is a big one for me, I used to write something, then rewrite it the “clever” way. Now I go in the opposite direction preferring more lines of code to keep the whole thing easily understandable.
I think the opposite of this will be in your next post, middle aged javascript 🙂 I chop and change as well but then when I come back to it later on it means more work to whip a bit of one project and re-use it in another… I think this one of our adolescent mistakes!
Re: the let vs var debate going on,
JSHint already checks that you don’t use a variable outside of the block it was declared in. So you could literally do a find and replace var with let safely, provided you ran JSHint first and fixed all those cases. I would imagine there wouldn’t be many anyway.
Very nice article! BTW, one’s personal JS coding style can be visualized and compared at http://jsstyle.github.com/.
I went through this exact-same whole ‘growing up’ experience in 1972!
Great post, thanks.
For “Strict coherence to a standard,” did you mean “adherence to”?
@Kyle
I believe variables should be declared close to their first usage and find this increases readability (unlike declaring them at the top).
Also, if declare a variable inside a block then use it outside, you deserve to have things break when it is later changed to let. In short, my experience says that a variable should only live as long as it needs and its declaration should be included in such.
If your variable is used outside a block, then it should be declared outside the block. And from my view, allowing any other thing is a sign that the language could use refinement.
Another concept that bolsters the practice of declaring variables at the top of a function is that functions should not go on forever — over on the compiled software side, there are a lot of folks who say that a function definition should fit on a single screen. I’m not that adamant, but when your function fits on one or two screens, variables declared at the top are no longer remote from where they’re used — and it’s much easier to identify variables which are typo-undeclared.
I agree with most of this except the strict equality and using logical operators. Like Steve said pressing one more time equals sign doesn’t hurt and you free your mind of thinking whether it’s actually needed. When it comes to logical operators your example is not a common real world scenario one. I think you’d rather see something like
or
Also I think you should have mentioned the dreaded semicolon. Which, again, is not necessary, but for the peace of mind it’s just easier.
Think of the following:
It may not be the standard idiom for somebody used to C/C++/Java, because of how scoping works there, but it currently works in FF, Opera and Chrome (I can’t run IE on Linux, to test there too), and it’s justifiable, since it makes the code more compact and sticks to the good habit of declaring a variable as close as possible to where you need it. Replacing
var
withlet
should break it.Therefore my approach is to cross a bridge when I get to it. For now I’ll stick to
var
for local variables, keep declaring variables as close to where they’re used as possible, not worry about scoping at block level, and rely on jslint and unit tests to catch any problems oncelet
becomes usable.I also don’t buy into using strict equality just to avoid not using it occasionally, when it is actually needed. As a programmer, I’m supposed to know what I’m doing, and unit tests should catch such problems, or they’re not correctly done.
“Commenting nothing shows hubris and laziness.” is IMO a bit strong of a wording. I try hard not to comment anything – I re-read my code, and if it seems easy enough to understand, I don’t comment it, if not, it probably needs improvement, not comments. IMO comments are useful only for rare, exceptionally non-obvious things. If your code is full of such things, it definitely needs refactoring/improvement.
I totally agree about the long names, except for unit tests. There, long test names do make sense, IMO – unit tests also serve as documentation, remember? OTOH, you don’t always want to use classes to avoid using long names. Sometimes, even if it’s just syntactic sugar, it makes sense to use namespacing, like for (a trivial/naive) example:
This avoids name clashes even when short names are used and is explicit and documenting. Of course, exaggerating leads to unwieldy method calls (i.e. like
my.very.deeply.nested.name.space.and.then.some.someFunction()
).I’ve never been an expert at JavaScript even though I’ve used it for years. I’ve read great insights both in the article and the comments, especially Avron’s naming system, as one of the most overlooked aspects of coding is readability, for you and your potential successors… in this light, I prefer to have variables declared at the beginning of a function, unless the function is a very long and convoluted one.
@Anonymous Coward
That example of declaring the variable twice is kind of ugly. Better to do something like this:
Cleaner and unbreaks the code in case of the global S&R with “let”.
While I’m personally not a fan of the “list all variables at top of function scope” convention, there’s another good argument for it: better gzipping. The variables listed at the top of the function scope will be replaced first by the shortest possible replacements: a, b, c, etc. This micro optimization is used by libraries such as jQuery when every byte counts.
Overall, however, I don’t think it’s valuable to be so prescriptive about where variables are declared, as there are good arguments on both sides. I’ll just say this: be clear, not clever.
The worst is:
typeof foo === 'undefined'
Sure, undefined can be overridden, but hell even Crockford just uses a straight comparison.
I couldn’t agree less Maxine. Avron’s convention is just plain wrong, perhaps with the exception of some sort of identifier for global variables. Code readability is not about a private system of type and scope tracking. It is about good naming (widely regarded as the hardest task in programming) and well structured code.
The prolific misuse of Hungarian notation to specify the type of the variable and not the kind is in and of itself plain awful. Putting this on steroids with additional scope information is just madness.
You may have taught your brain to parse as_ meaningfully into ‘argument of type string’ but for the next poor soul to pick up your code it simply says ‘as’. My brain reads ‘as firstname’ which makes little or no sense at all, in fact it almost infers some entirely different and unintended meaning.
If you can’t understand the scope of a variable from where it is being used combined with some standardised casing conventions then it’s probably time to refactor. Same argument applies to Kyle’s logic that it is better to pre-hoist all your vars.
The chief value of declaring variables at top of scope is for my own sanity when writing functions (and that of my future self and others interpreting my code): how many moving parts are in this act? Who are the players? It helps clarify wonderfully.
If there are too many, I’ll usually see that a few of them are incongruous or superfluous, and refactor into multiple functions; especially during debugging, stepping through the stack, I like to have a name for my function which says what it does, and names for the little things involved in that. If there are more than eight, I’m probably doing too many things at once.
Regarding the `let` argument, I don’t want to modify my code on the basis that an inheritor of my code will do something immensely stupid and not have the intelligence to then immediately realise what it was they did that was so stupid. If they are that stupid, I would rather they learnt the hard way! In any case, moot point 😉
James, great post about rethinking your positions on occasion.
Realizing that people have preferences, and realizing that I routinely write JS that doesnt use strict equality, I’d just like to defend strict equality a bit.
Your example, if it uses ===, doesn’t need the typeof check. That, to me, is the power of strict equality: Concise expression of intent. The number of gotchas for strict equality is demonstrably lower than loose equality.
I’m not saying to never use it. Look, sometimes, if it sounds like a duck, it’s a duck. And there are certainly cases where I use loose equality:
prop == null; // matches null or undefined
elem == elem.window; // detects if the element is the window
But these cases require commenting or explanation to people. Case in point, I see a lot of this:
prop == null || prop == undefined; // the undefined check is useless here, and its why JSHint has the eqnull option
People should find their own way, but there is a difference between opinion and opinions formed through experience. Thanks for reminding me of this.
@Mike Sherov, My example for when strict-equality isn’t necessary, in hindsight, was probably the worst example I could have chosen, because it actually ends up being proof of strict’s benefits. What I had intended was to show how, if you know the type of something, for certain, such as return values of typeof, then you don’t have to do a strict-equality check. Also,
foo==null
is damn useful. Most of the time I used strict, but there are times that I wish a simple non-strict wasn’t so taboo.Richard: There is another reason for preferring
typeof
forundefined
checks, which is that, unlike comparing toundefined
, it is guaranteed to work in all circumstances, including checking host object properties.For example, the following is used in IE for parsing XML:
var x = new ActiveXObject("Microsoft.XMLDOM");
To check whether it has a
loadXML
method safely:typeof x.loadXML === "undefined"; // Returns false
On the other hand:
x.loadXML === undefined; // Error is thrown
Really great article made me think about some things. However, I wouldn’t say consistent strict equality (===) is immature, rather it’s a discipline worth keeping. In your example its clearly unneccessary, but that shouldn’t make it an unneccessary practice to maintain in consideration to the whole language. Once you start using == alongside === it can become confusing for others whether you meant === or == (not in the case of your example, but potentially in other cases).
<3 but small nit 😉
Under "Using logical operators as statements", that was never a statement, always an expression 🙂
I’ve been programming since 1966 and I approve of this message.
…except for unit tests. There, long test names do make sense, IMO – unit tests also serve as documentation, remember?
I agree that unit tests serve as “one form of” documentation; however, I wouldn’t go so far as to say that long names are necessary…they are an implementation detail in cases where the testing library falls short in allowing expressive descriptions.
The goal is to have descriptive testing specs that read as sentences. Inflecting the function name into a description is a legacy implementation detail that need not be cargo-culted in flexible languages (such as JavaScript).
When using a test framework that allows separation of description from test specification, the argument that long test names are necessary (or even good) becomes moot.
…never solves what you think it will…steer clear of over-zealous coding style dogma
I agree 100%. Dogma, cargo-culting, and complacency are developer growth killers.
I agree that foo==null is damned useful, that’s why I said I use it 🙂
Aren’t your opinions on “Strict equality everywhere” and “Assuming truthiness equals presence” coming from opposite sides of the you-know-what-you’re-doing-but-just-don’t-mess-up spectrum? I’m not objecting, but I’m wondering how you reconcile that.
I’m a big fan of
Readable, and efficient. 🙂
@Buggy Bug
“I consider this an extremely weak argument. Code that breaks because of the replacement of var with let, is code already broken logically. Why was the coder using a variable outside of a block that he declared it in the first place? I can’t think of any reason, besides bad design. ”
This argument is a lot stronger than you agree with. First and foremost javascript allows the use of variables outside of a block scope. But essentially are these variables outside their scope? My answer is no. Note, javascript allows variables to be used; “outside of block scope”, which doesn’t mean(or better put is not equivalent to) “outside of scope”. Javascript expands the scope of a variable declared in a block, beyond the scope of the block(contrary to what exists in most other programming/scripting language idioms).
Of course this is a bad design, but it is accepted javascript coding practise. And I can say at the minimum 10% of javascript code in live, distribution(libraries) and production, have codes written this way. And for no other reason, than because javascript allows it. Once existing code is rewriten by replacing “var” with “let”, of course bugs can and will creep into code written this way.
Qouting Douglas Crockford; “In most languages, it is generally best to declare variables at the site of first use. That turns out to be a bad practice in JavaScript because it does not have block scope. It is better to declare all variables at the top of each function.” — Awful Parts: Appendix A – JavaScript: The Good Parts
That is why why , @James gave the title of this article as “JS adolesence”. Javascript experts or wizards anywhere else in the known universe won’t have a problem with this. But there is little argument that the concept is clear, practical and objective. And a few coders may have developed such tewwwwible habits (like @james emphasized).
Or/and would you be unwilling to also accept that the ECMAscript Sages chose to introduce the “let” keyword, partly for this same reason????
For example;
@James, fine article.
Thanks
Heard about this on The Javascript Show Podcast, nice post & thanks for the tips.
Agree with all except this one:
In very few situations this is dangerous; it only can become an issue in for-in loops.
I have always over-commented my code till the moment my co-worker said it’s because I write ugly and unclear code. Since then I started to focus more on the code’s aspects.
This is a really, really great article. It does an awesome job of balancing pragmatism versus good coding. Too often language debates turn into name-calling and nitpicking over semantics, but your article really demonstrates the difference between a junior developer and a seasoned vet. This is required reading for my team now. Thanks!
Interesting article. It would be useful for these types of discussions to also include an example of what is right.