One big problem with the sample code is that making XMLHttpRequest.send synchronous means all JS execution must pause while waiting for the request to be received. There's no reason not to use an asynchronous call.
Asynchronous calls can improve responsiveness, but what they don't give you is coordination, which means a task won't run until the data it needs is ready. The standard way of coordinating asynchronous code is to pass to the asynchronous function a function that, when executed, performs the rest of the computation that relies on the data. This function has the technical name "continuation", which is simply a function that represents the rest of the computation from a given point forward. That is, turn:
f1();
f2();
async();
f3();
f4();
into:
f1();
f2();
async(function() {
f3();
f4();
});
Because you're passing around a continuation, this is known as "continuation passing style". XMLHttpRequest is a special case in that rather than passing a function to the asynchronous function, you set it as a listener for the readystatechange event on the XHR object. That is, you assign the continuation to xmlhttp.onreadystatechange.
There are a few more improvements to make. First, add error detection. The status property of the XHR instance holds the HTTP status, which you can use to check for errors.
As a number of others have mentioned, eval can be problematic and should be avoided when there's another option. For one thing, you have to make sure the string comes from a trusted source. The particular problem with eval here is that the script is evaluated in the same context as the call to eval. If the eval happens inside a function, anything defined by the script isn't visible outside the function. If your script doesn't need to define anything (and will never need to define anything; always consider the future of your code), you can use eval. Otherwise, dynamically create a script element with the script as content and add it to the document; you can define a function that does this (see globaleval in the sample below).
xmlhttp is a global variable, which is bad. Instead, declare it as a local variable.
Rather than setTimeout, which is for one-shot calls, use setInterval, which calls the passed function periodically. Note that both setTimeout and setInterval may take longer than the given delay to run, though that shouldn't be an issue here.
(function () {
// keep variable from polluting global namespace
var showpart2Interval = 0,
scriptElt = {parentNode: {removeChild: function() {}}};
function globaleval(script) {
scriptElt.parentNode.removeChild(scriptElt);
scriptElt = document.createElement('script');
scriptElt.type = 'text/javascript'
scriptElt.appendChild(document.createTextNode(script));
document.body.appendChild(scriptElt);
}
function showpart2(){
var xmlhttp = new XMLHttpRequest();
xmlhttp.open("GET","atuamae.org/parte2-encomendar.php",false);
xmlhttp.onreadystatechange = function() {
if (xmlhttp.readyState == 4) {
if (200 <= xmlhttp.status && xmlhttp.status < 300) {
globaleval(xmlhttp.responseText);
} else {
// HTTP error
...
}
}
}
xmlhttp.send(null);
}
function startShowpart2() {
if (window.XMLHttpRequest && !showpart2Interval) {
showpart2();
showpart2Interval = setInterval(showpart2, 15000);
}
}
function stopShowpart2() {
clearInterval(showpart2Interval);
showpart2Interval = 0;
}
window.startShowpart2 = startShowpart2;
window.stopShowpart2 = stopShowpart2;
})();
startShowpart2();
If you don't care about implementing all of this yourself, have jQuery do the heavy lifting. It's good to know how to do things yourself, but (for production code) using standard libraries with standard interfaces speeds up development in a number of ways.
See also
eval.