Eval as part of ajax shell.. bad idea?

I’m just learning ajax and have been using a pretty straightforward shell for writing the response text on state changed:

document.getElementById("htmlid").innerHTML = xmldoc.getElementsByTagName("xmltagname")[0].childNodes[0].nodeValue;

A friend showed me a way where the state changed function is basically just

eval(xmldoc.getElementsByTagName("ajax")[0].childNodes[0].nodeValue);

and then you just build the ajax tag in the php file adding to it all the javascript document replaces etc using a function. It is actually much quicker to code because everything is in the same document. Plus it allows me to write javascript out to the page and have it actually run (I wasn’t able to get the ajax to send you to a new page otherwise)

I’ve heard people being down on eval though because it can be slow and risky security-wise. But also seen that this is the one place where it’s ok/good(?) to use.
Should I be concerned about that? This is for a form that accesses and writes to a database, but it’s all done through php and none of that php is returned back to the js file, only innerHTML and document.replace.

I know there’s not necessarily one ‘right way’. But I’d be interested on opinions, because I like the idea of best practices but as a newcomer I find it difficult to know/evaluate what is

curious to get some opinions here…? it’s a longtime programmer’s recommendation against my teacher’s, and I’d just like to see some thoughts, since right now I’m sort of unsure which way to favour.

Just so I get this right… is there JavaScript with “document replaces” in your XML doc that you want executed?

It might be better to have your AJAX request fetch a set of data or markup (whether that be JSON or XML) that then replaces content. So you have all of your JavaScript in one place and your data in the other.

Would you be able to give some code samples of what you are retrieving in your ajax request?

sure thing!

in the version with eval:

Javascript file:

function registerChanged() { 
if (results.readyState==4 || results.readyState=="complete") {
xmldoc = results.responseXML;

  eval(xmldoc.getElementsByTagName("ajax")[0].childNodes[0].nodeValue);
	} 
}

php file:

//conditionals setting error output etc.
function replace_html($dom_id, $data) {
  return "document.getElementById(\\"".$dom_id."\\").innerHTML = \\"".$data."\\"; \
";
}

$ajax ="";

if ($flag > 0) {
$confirm = "There are errors with your submission.";

$ajax = replace_html('fnameError', $errorfname);
$ajax .= replace_html('lnameError', $errorlname);
$ajax .= replace_html('emailError', $erroremail);

//...etc


echo "<ajax><![CDATA[".$ajax."]]></ajax>";


Version 2 works roughly the same way except all the javascript is just written right in the js file via xml created in the php file

function registerChanged() { 
if (results.readyState==4 || results.readyState=="complete") {
xmldoc = results.responseXML;

document.getElementById("fnameError").innerHTML = xmldoc.getElementsByTagName("fnameerr")[0].childNodes[0].nodeValue;
document.getElementById("lnameError").innerHTML = xmldoc.getElementsByTagName("lnameerr")[0].childNodes[0].nodeValue;

etc

php:

//conditionals setting error output etc.
	
echo "<messages>
		  <fnameerr>".$errorfname."</fnameerr>
		  <lnameerr>".$errorlname."</lnameerr>
		  <emailerr>".$erroremail."</emailerr>
		  <usererr>".$erroruser."</usererr>
		  <passerr>".$errorpass."</passerr>
		  ";

that’s not everything but should give a pretty good idea of how it functions. version one sort of makes the complexity of creating the xml etc superflous because you just build your javascript right out of the variables created in the php and then just spit it out.

Cool, I thought that was what you meant. Just wanted to be sure.

From an application structure and maintenance point of view, the method in which you store JavaScript to modify the document is definitely less than ideal, in that it goes against the point of having your application return data wrapped in XML.

My reason for being against the eval() for that reason is not because “eval() is evil” but because in this case it presents an issue with the flow of your program.

It would be much more beneficial to keep the XML as your data provider and the JS as your logic that manipulates that data. This will mean that if you (or someone else) needs to add a field they can do so within the XML and add, if required, a little bit of extra code to the logic to manipulate it.

you could use a simple function in JS that would be similar to your “replace_html()” function.

function changeHTML(elementToChange, fromXmlElement) {
  document.getElementById(elementToChange).innerHTML =
     xmldoc.getElementsByTagName(fromXmlElement)[0].childNodes[0].nodeValue;
}

Additionally, and this is really more a personal opinion, I prefer returning JSON objects when returning server data to JS as it feels a lot more natural to work with something that can really easily be converted to a native JS object. JSON.org has some nice parsers available for both PHP and JS.

P.S.: I could have sworn I wrote a reply and submitted it yesterday, but alas it’s not there… I might have accidentally closed a tab lol.

Thanks for the reply!
Although… I guess I don’t totally buy how the second one is more straightforward.
Like if I conditionally want to run javascript, the eval way, I just add it onto my $ajax variable.
The js in the js document way (and I’ll admit there’s some logic to that at the very least), I have to make the php write a conditional that gets sent to the js document via a made-up xml tag and then set another conditional in the javascript to decide whether to execute the code…

for instance in a login script…

in the php file:


else //valid registration conditions not met 
{	
//adding user data stuff to db snipped
	
$ajax = "window.location.replace(\\"http://localhost:8888/test/\\")";}

and no changes necessary to the js.

whereas for the second method, to redirect the page I have to make a different variable, say $redirect, define it to be NULL normally, but if registration successful make it ‘yes’ or maybe the page I want to redirect to, then echo"<redirect>“.$redirect.”</redirect> or what have you. Then I have to go to the js document create a line of code that checks if redirect is full, or if it’s yes and if so execute the javascript.

Am I missing something? because the second way seems much more complicated to me…

(not to mention potentially having to set up separate variables for javascript vs php, since in the case of a redirect frex, I need an absolute path, therefore making some kind of a $siteroot variable very useful).
I know it isn’t exactly using XML… but we were taught that the reason to use XML is really just to allow you to return separate things from the php to put in different places in the page. Perhaps there’s a benefit to xml I’m missing.

I guess my main point about method #2 was that you keep everything separate.

but we were taught that the reason to use XML is really just to allow you to return separate things from the php

XML should be used as as a data transportation method and wrapper to help describe the data that you are returning. This allows you to access that data using not only with JavaScript but other languages as well.

When you put both the data and the code that performs manipulations in the data layer you lose the ability to later on access that data in a different way.

While it sometimes seems like it’s more work to code in a manner where things are separated out, in the long run you’ll see the benefits when you don’t have to change your old code (and potentially break things), but when you can simply add a method to perform manipulations on the data.

As a rather simplified analogy: This is one of the reasons why we don’t use inline-styles in HTML, because if we want to change something later on it might mean changing 100 documents, rather than 1 stylesheet.