Jump to content

Is this the most efficient way to do this?


caruga

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 comment
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 comment
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 comment
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
×
×
  • Create New...