Element declared as variable vs get element?

Question

So I have this function, since I don't use JQuery:

var $ = function(id) {return document.getElementById(id)};

Now let's say I want to use this in a function to for example remove an element's all children:

function clearMessages() {
    while ($("messages").lastChild) {
        $("messages").removeChild($("messages").lastChild);
    }
}

Is this "bad code" that is very slow compared to:

var messages = $("messages");

function clearMessages() {
    while (messages.lastChild) {
        messages.removeChild(messages.lastChild);
    }
}

? Because of the fact that it runs the document.getElementById fn quite a few times every time I use the clearMessages fn?

You can also write the function as `var $ = document.getElementById;`. — Sebastian Simon, Dec 17 '15 at 18:12
@Xufox Obviously I have tested that (which doesn't work), but thanks anyways and as Bergi says, only if you add `.bind(document)` — , Dec 17 '15 at 18:18
One real quick and simple way to empty out the `messages` element is just: `$("messages").innerHTML = '';` — neilsimp1, Dec 17 '15 at 18:27
@neilsimp1 That is one real slow way, http://stackoverflow.com/questions/3955229/remove-all-child-elements-of-a-dom-node-in-javascript — , Dec 17 '15 at 18:28
@Murplyx Huh, go figure. Well, from a code standpoint it's quick at least. Just not performance-wise. — neilsimp1, Dec 17 '15 at 18:30
@neilsimp1 Then it is one real slow way, but one simple way as you said. — , Dec 17 '15 at 18:32

score 1 · Answer 1 · 2015-12-17T18:50:26.730

The first problem with both your examples is that they are hard-wiring references to either the id #messages or the DOM element messages. That's easy enough to solve in the first example, by making id a parameter. Since it's no longer specific to messages, we'll also rename it:

function emptyById(id) {
    while ($(id).lastChild) {
        $(id).removeChild($(id).lastChild);
    }
}

It's obvious that one does not want to look up the element over and over again. Beyond performance, it is duplicative and error-prone. Therefore, it should obviously be rewritten as

function emptyById(id) {
    var elt = $(id);
    while (elt.lastChild) elt.removeChild(elt.lastChild);
}

However, the problem with this approach is that it mixes two entirely separate problems:

Finding an element with a particular ID.
Performing some operation on this element (in this case, removing its children).

In practical terms, assume you have an element, whose children you want to remove. Given the above design, you are forced to say

emptyById(elt.id)

which won't even work if the element does not happen to have an ID.

Therefore, it is much better design to follow your second example, and define the interface via the DOM element not its ID (rewriting the loop slightly):

function emptyChildren(elt) {
  var last;
  while (last = elt.lastChild) elt.removeChild(last);
}

Now, if you have the element you can simply call emptyChildren(elt); if you have the ID, you can call emptyChildren($(id)).

None of this is really about performance. getElementById is fast enough that you would not notice if you called it ten times when you really only needed to call it once--assuming, of course, that this is not in a tight loop being executed one million times. The important question is how to design your interfaces to be general and flexible.

Now I really wonder why you'd call that function `clearMessages` when you want to decouple it from the id `messages`… — Bergi, Dec 17 '15 at 18:40
Why declaring `last` when elt.lastChild is already set and is not a function? — , Dec 18 '15 at 08:17

Element declared as variable vs get element?

1 Answers1