0

I have seen and avoided code like this:

<div id="myDiv" onClick="alert('hi!');">Div</div>
<script>
    myDiv.click();  // <-- this bit seems wrong
</script>

It just seems wrong to me that I should refer to the node by it's id alone without explicitly assigning it to a variable first. Like this:

var myDiv = document.getElementById('myDiv');

Is there a good reason why we shouldn't use code like in the first example or am I just superstitious?

Jack Allan
  • 14,554
  • 11
  • 45
  • 57

3 Answers3

1

It's bad practice to use the identifiers from the page without specifying where they come from.

The identifiers are "magically" created as properties in the document object. If you use those, it's not clearly apperent how they got there in the first place. If you rename the element, the same Javascript code now creates a global variable instead, and it's hard to see by looking at the code what it was supposed to do, and why it's not working any more.

Also there is a risk for conflicts. If you create a global variable with the same name as the element id, then it will shadow the element and you can no longer reach it.

By using getElementById the code clearly says that there should be an element with that id in the page, and it also only looks for an element so the risk for naming conflicts is less.

Guffa
  • 687,336
  • 108
  • 737
  • 1,005
  • 1
    Why the downvote? If you don't explain what it is that you think is wrong, it can't improve the answer. – Guffa Jul 07 '14 at 11:20
0

Thanks to Bergi and Pete for links to useful information on this topic.

From these I have gleaned there are few main reasons why this is bad practice:

  1. It's not supported in all browsers (although with html5 adopting implicit references this approach this may not be an issue going forward).
  2. Namespace collision - the reference is put as a property on this window object. If a window property with a name the same as the id exists it may not be assigned.
  3. Due to point 2, your code could break unexpectedly if a browser introduces a property with the same name as one of your ids.
Jack Allan
  • 14,554
  • 11
  • 45
  • 57
-3

First of all, it is a bad practice since the maintenance of such code will be very difficult, consider that the script is not located next to the element or even not in the same file.

Moreover, using ids in general is not recommended when your HTML page/app grows. The id attribute must remains unique and you may find it hard to follow it

Ziv Levy
  • 1,904
  • 2
  • 21
  • 32
  • Who on earth is recommending not to use IDs in general? – Anthony Grist Jul 07 '14 at 08:42
  • 1
    using `id` when having 'many-pages' app, multiple events, data-binding and auto-generating template mechanism, you would rather avoid using an `id` attribute. I also mentioned that *it is not recommended* when the app grows, as general comment – Ziv Levy Jul 07 '14 at 08:59