Jump to content

Is this the most efficient way to do this?


Recommended Posts

Knowing several languages but rusty with javascript, I know how there can be pitfalls in learning bad habits or coming up with convoluted, less than optimal solutions that would make more sense in other languages, almost abusing the language rather than working off of what it does best. So I am wondering if anything is unoptimal with the code below. The xml file is being read in; the code expects a 'featurelist' tag where 'ideas' and 'chapters' are organised under other chapters (except the top-level ones). Right now the code doesn't acknowledge the idea tags, it just tries to render the chapters in a html 'table of contents' in a tree-like organisation. The outputted html is correct, but I inserted some extra code to skip tags of the wrong id. This is the particular hot-spot where I'm wondering if better solutions are available, perhaps built-in rather than writing it 'by hand'.

<script type="text/javascript">function processxml(xmldoc) {  i = 0;  root = xmldoc.getElementsByTagName("featurelist")[0];  el = root.getElementsByTagName("chapter")[0];  /* Initiate contents */  str = '<ul id="contents">';  while(el && el != root) {	if(c=el.getAttributeNode("htmlclass"))	  str += '<div id="' + c.value + '">';	str += "<li>" + el.getAttribute("title") +"</li>";	if((els = el.getElementsByTagName("chapter")).length) {	  /* Has child chapters */	  str += "<ul>";	  el = els[0];	  continue;	}	esc = 0;	while(!esc) {	  save = el;	  while((el = el.nextSibling) && el.nodeName != "chapter")		;	  if(!el) {		/* No next sibling */		str += "</ul>";		el = save;		while((el = el.parentNode) && el.nodeName != "chapter")		  ;		if(!el)		  /* No parent */		  break;		else		  /* Found parent, look again for sibling */		  continue;	  }	  else {		/* We have next sibling */		if(c)		  str += "</div>";		esc = 1;	  }	}  }  /* Terminate contents */  str += "</ul>";  alert(str);//  document.write(str);  document.body.innerHTML = str;}

Or have I done everything as well as it could be done?

Edited by caruga
Link to post
Share on other sites

The main problem I see is that all of your variables are global. You should define variables using the var keyword to make them local to that function. You can put all of them on one line or statement:

var i = 0,  root = xmldoc.getElementsByTagName("featurelist")[0],  el = root.getElementsByTagName("chapter")[0],  str = '<ul id="contents">';

Link to post
Share on other sites
The main problem I see is that all of your variables are global. You should define variables using the var keyword to make them local to that function. You can put all of them on one line or statement:
Okeydoke. Since writing my 1st post I discovered 'jquery'. This seems to do what I had in the back of my mind as a 'convenience' function. I wonder if it can express in much fewer lines of code what the code in my OP does... I gather from the code you wrote that all statements are also expressions (i.e. the variable declaration can be next to a comma operator), unlike C. Next thing I'm looking at is having all the ideas integrated into the visible tree structure (with ever-decreasing font-size, down to the microscopic), and being able to click on any chapter and having it zoom (and animate the zooming) in scale with the chapter depth clicked on... Edited by caruga
Link to post
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

Loading...
×
×
  • Create New...