It started like this: I was reading some JavaScript code, written – he says charitably – some years ago. Much simplified, it looked something like this, with all identifiers renamed to protect the guilty:
timer = {};
timer.timerID = null;
timer.cancel = function () {
if (timer.timerID !== null) {
clearTimeout(timer.timerID);
timer.timerID = null;
}
};
timer.start = function (delay, onDone) {
timer.cancel();
timer.timerID = setTimeout(onDone, delay);
};
Let’s just analyze this for a moment. Yes, it’s creating a global object and then adding a bunch of methods and properties to that object. At one level then, it’s good: it minimizes the impact on the global namespace since there’s just that one global variable. And, er, that’s it. The biggest thing for me on reading this for the first time is the complete lack of closures and local variables. From that insight come two major issues: first, there is no data hiding at all. Second, this is a singleton, or, if you like in C# terms, it’s a class that acts as an already-instantiated object. In other words, there is no chance of having two timer
objects. Let’s address these two points separately.
Regarding the first point, to me this code is calling out for an IIFE, an Immediately-Invoked Function Expression. This is a piece of code that is structured as a function (actually a function expression), one that is invoked as soon as it’s encountered.
(function () {
var timerID = null,
cancel = function () {
if (timerID !== null) {
clearTimeout(timerID);
timerID = null;
}
},
start = function (delay, onDone) {
cancel();
timerID = setTimeout(onDone, delay);
};
timer = {
start: start,
cancel: cancel
};
})();
(Notice that the function expression is anonymous and that it is enclosed in parentheses. The reason for this latter restriction is to avoid the case that the interpreter confuses this IIFE for a function declaration.)
Anyway, in doing this, I have automatically created a closure and therefore already have some hidden local variables. I then restructured the code to take advantage of these ‘private’ variables and make the whole thing cleaner and clearer. Except there’s a subtle bit of code “misdirection” in there that, the first time I coded it, I didn’t have and the code didn’t work. What is it? Well, notice I’m declaring all the local variables of the IIFE as a single, continued var
expression, with variables separated by commas? In my original code, I didn’t terminate this expression after the start
function and then thereby made timer
also a local variable. The only difference between the “bad” code and the “good” code is replacing a comma with a semicolon. Not exactly really obvious what’s going on, eh?
Better might to be more explicit:
window.timer = {
start: start,
cancel: cancel
};
And then I remembered a trick I’d learned a long time ago. Not a lot of people know that the this
keyword for an IIFE actually refers to the global object, or window
– the interpreter assumes that such anonymous functions are called as if they were methods on the global object. So we don’t in fact have to refer to window
in our code. This is handy in those cases where we’re writing JavaScript code for, say, node.js, where the global object is not called window
.
this.timer = {
start: start,
cancel: cancel
};
This all worked until I got conscientious and added "use strict";
at the top of the file to turn on strict mode. Boom, the code stopped working again: “this is undefined”. Eh, what?
It turns out that my old “trick” was one of those things I should have immediately ignored and forgotten (as will you right now!). This “if a function is called outside of an object context, set this
to the global object” was never part of the JavaScript standard, and the newer strict
rules expressly state that this is not so. In these cases, this
is expressly set to null
. Back to window
then, or perhaps something like this where we pass in the global object explicitly:
"use strict";
(function (globalObj) {
var timerID = null,
cancel = function () {
if (timerID !== null) {
clearTimeout(timerID);
timerID = null;
}
},
start = function (delay, onDone) {
cancel();
timerID = setTimeout(onDone, delay);
};
globalObj.timer = {
start: start,
cancel: cancel
};
})(window);
However, at this juncture, it starts to really make sense to take account of my second point above: why have this as a global object in the first place? Why not just make it a function that creates a new timer object: the code is almost all there anyway? (And, no, I don’t mean a constructor.)
"use strict";
var createTimer = function () {
var timerID = null,
cancel = function () {
if (timerID !== null) {
clearTimeout(timerID);
timerID = null;
}
},
start = function (delay, onDone) {
cancel();
timerID = setTimeout(onDone, delay);
};
return {
start: start,
cancel: cancel
};
};
Yes, this creates a global function, so we’re almost back to square one. Just add it as a method to a global object you have already created, say a utilities object.
In fact, you could go even further: why reuse a timer object? Just set it going and when it’s done throw it away. Need a timer again? Just create another timer. Oh, and while we’re at it, let’s have an onCancel callback, and use a configuration object and pass it to the onDone and onCancel callbacks. But that’s a story for another day.
No Responses
Feel free to add a comment...
Leave a response
Note: some MarkDown is allowed, but HTML is not. Expand to show what's available.
_emphasis_
**strong**
[text](url)
`IEnumerable`
* an item
1. an item
> Now is the time...
Preview of response