views:

232

answers:

7

Is there a more efficient way to write the following appendChild / nesting code?

var sasDom, sasDomHider;
var d = document;
var docBody = d.getElementsByTagName("body")[0];
var newNode = d.createElement('span');
var secondNode = d.createElement('span');

// Hider dom
newNode.setAttribute("id", "sasHider");
docBody.appendChild(newNode);
sasDomHider = d.getElementById("sasHider");

// Copyier dom
secondNode.setAttribute("id", "sasText");
sasDomHider.appendChild(secondNode);
sasDom = d.getElementById("sasText");

Thanks in advance for your time :)

+1  A: 

It's because newNode references an instance of a HtmlElement which you are attempting to insert into two different places within the DOM. You'll need to create a new element each time (or use cloneNode, but there are cross browser discrepancies with how that works).

Something like this should work

var sasDom,        
    d = document,
    docBody = d.getElementsByTagName("body")[0],
    sasDomHider = d.createElement('span');

// Hider dom
sasDomHider.setAttribute("id", "sasHider");
docBody.appendChild(sasDomHider);

// Copyier dom
sasDom = sasDomHider.cloneNode(true);
sasDom.setAttribute("id", "sasText");
sasDomHider.appendChild(sasDom);
// job done. sasDomHider and sasDom still reference the 
// created elements.
Russ Cam
Upped for accurately identifying problem. I'm now hoping to find the most efficient answer.
Matrym
+2  A: 

Ok, question has changed. Blah. Here's the new answer:

You might gain a little bit in the way of execution efficiency by building the branch before appending it to the DOM tree (browser won't try to recalc anything while building). And a bit in the way of maintenance efficiency by reducing the number of superfluous variables:

var d = document;
var docBody = d.getElementsByTagName("body")[0];

// Copyier dom
var sasDom = d.createElement('span');
sasDom.setAttribute("id", "sasText");

// Hider dom
var sasDomHider = d.createElement('span');
sasDomHider.setAttribute("id", "sasHider");

sasDomHider.appendChild(sasDom); // append child to parent
docBody.appendChild(sasDomHider); // ...and parent to DOM body element

Original answer:

You're trying to insert the same element twice, in the same spot...

var newNode = d.createElement('span');

...That's the only place you're creating an element in this code. So there's only one element created. And you insert it after the last child element in the body here:

docBody.appendChild(newNode);

So far, so good. But then, you modify an attribute, and try to insert the same node again, after the last child of sasDomHider... which is itself! Naturally, you cannot make a node its own child.

Really, you want to just create a new element and work with that:

newNode = d.createElement('span');
newNode.setAttribute("id", "sasText");
sasDomHider.appendChild(newNode);
// the next line is unnecessary; we already have an element reference in newNode
// sasDom = d.getElementById("sasText");
// ... so just use that:
sasDom = newNode;
Shog9
I'm hoping to end up with two dom elements, one nested in the other. Does this do that?
Matrym
Whoa... You re-wrote the whole question while I was typing this. Bah...
Shog9
Sorry Shog. I got a quick answer that made the error obvious - but not the best solution.
Matrym
Hum. Well, see edit.
Shog9
Do you have any thoughts on this vs using innerHTML?
Matrym
@Matrym: innerHTML can be faster, but also more fragile as complexity increases (you're responsible for escaping your own attributes, etc.) See: http://stackoverflow.com/questions/1694233/create-append-node-vs-innerhtml
Shog9
A: 

Generally one set of the body's innerHTML with the desired HTML be the most efficient method.

e.g.

document.body.innerHTML = document.body.innerHTML + '<span id="foo"></span><span id="bar"></span>';
wombleton
If I do it this way, can I still work with these spans as dom elements by getElementById methods? Are there any drawbacks to using innerHTML? Many people seem to prefer to create dom elements, but I'm not totally sure why.
Matrym
As soon as they are created they are indistinguishable from elements created by any other method. Pretty sure the libraries prefer innerHTML for reasons of speed.
wombleton
Be careful with document.body.innerHTML = document.body.innerHTML + ... because that means that all the content of the page is serialized to string and back to DOM, and it can be very expensive. Usually people creates a div that it's inserted into the DOM and set the innerHTML of that element, that way you avoid this penalty
AlfonsoML
This is very far from efficient. It's true that `innerHTML` is fast for setting the contents of an empty element, but appending to existing `innerHTML` like this inefficient, for reasons that AlfonsoML gave. Also, there are inconsistencies in how browsers implement `innerHTML` so this needlessly introduces risk of error.
Tim Down
A: 

In your example, the objects referenced by newNode, sasDomHider, and sasDom are all the same, all pointing at a single DOM element. You're trying to put it in two places at once. You need to clone it, or simply make a new <span> for the second instance. Merely changing the id attribute is not enough (you're changing the id of the one already in the document).

bcherry
+1  A: 

You don't need to search again for the nodes:

var d = document;
var docBody = d.body;
var sasDomHider = d.createElement('span');
var sasDom = d.createElement('span');

// Hider dom
sasDomHider.setAttribute("id", "sasHider");
docBody.appendChild(sasDomHider);

// Copyier dom
sasDom.setAttribute("id", "sasText");
sasDomHider.appendChild(sasDom);
AlfonsoML
A: 

There are a few ways to make this more efficient (in terms of performance and code size/readability), most of which have been covered already:

// Hider dom
var sasDomHider = document.createElement('span');
sasDomHider.id = "sasHider";

// Copier dom
var sasDom = document.createElement('span');
sasDom.id = "sasText";

sasDomHider.appendChild(sasDom);
document.body.appendChild(sasDomHider);
  • Obtains body using document.body
  • Uses only one variable each for the nodes you've created
  • Removes the getElementById lines, since they get you references to the same elements you had already
  • Uses the id property of the elements rather than setAttribute, which is an unnecessary function call and more verbose
  • Creates the whole branch being added to the document before adding it, thus avoiding unnecessary repaint/reflow
  • Removes d as an alias for document: there's no need to keep another reference to the document hanging around
  • Removes the docBody variable, since you're only using it once
Tim Down
A: 
var sasDom=m$(document.getElementsByName("body")[0]).
    a$("span", {"id":"sasHider"}).a$("span",{"id":"sasText"});

You need functions m$ and a$

pkario