Error: "function is not defined"

[MoreEle]

Hello!
I'm getting this error in firebug:

start is not defined
[Break on this error] window.setInterval("start()",100);\r\n

I cant understand why, but i believe there is something wrong with the "window.addEvent('domready', function() { ##js code## });" because the loop works outside the domready event. Please help me.


The code:

window.addEvent('domready', function() { 


function start() {
    
 
    
    var text = $('textfield').value;
	
	

    
    if (text != comparevar) { // om det finns ny text >>
		
		
		var url = 'return.php'+'?sendtext='+text;
		
        new Ajax(url, {
		method: 'get',
		update: $('statusdiv')
	}).request();

		
		
    } else { //inget ska hända här....
		
    }
    comparevar = text;
    
	setInterval("start()",100);
    
}

var comparevar = $('statusdiv').innerHTML;


start(); 

});

[A1ien51]

well you definitely do not want to use setInterval there, if you get it running you would see why. You will get 1000s of "threads" runnning at the same time. You want to use timeout.

You have scoping issues since start is relative to a function and the interval is being fired at the window level and has no clue what start is. Using a closure or the function reference should solve it.

setInterval("start()", 100);

should be

setTimeout(start, 100);
or
setTimeout(function(){ start(); } , 100);

Eric

[Dimitar]

no no no! this looks like mootools 1.11...

first off, in mootools you should use .delay() or .periodical and not setTimeout and setInterval (wrappers that are chained and without string eval and support scoping - eg, function.delay(nnn, this); ). as alien51 pointed out, if you must use setInterval etc, drop the quotes or use an anonymous function if you have to, else a new instance of the js interpreter needs to be launched to figure and evaluate your string, big performance hit and bad practice.

second, don't use any of that anyway is a bad idea to organise the repetition in such a way...

now, you mention a problem with the domready firing--it won't be as it fires start once just fine ok and does try to run start() through setInterval, which comes as undefined in the global scope.

the problem:
setInterval("start()",100);

start is defined within the anonymous function for domready. move it outside so the instance of the js interpreter that runs the setInterval code can access it. as a whole, there can be problems with defining functions within functions that affect scope as mentioned above, in particular since the setInterval here has none (so, Window is implied). with start not defined there but within the anonymous domready wrapper, it just won't work...

anyway, none of that matters much - you should refactor things slightly for mootools anyway...

PHP Code:

// this can be used by start to cancel repetition (check bottom) later
var myTimer, start = function() {
   // define stuff. no self calls to as it creates an infinite loop. 
   // if you want to stop due to some response coming back or user action
   $clear(myTimer);
};

window.addEvent("domready", function() {
    start();
    // or if you want to wait anyway
    (function() {
        start();
    }).delay(100);

    // if you want to run it every 100ms then do this instead:
    myTimer = (function() {
        start();
    }).periodical(100);
}); 


i would also add, with ajax being asynchronous - it can be delayed etc and returns can badly overlap.depends largely on latency and speed of server etc, 100ms is just way too short a timespan, most GET requests within the visitor's own country finish within 100-150ms unless there's little data and no waiting.

another thing you should consider is the onComplete event on the ajax object, you can call start() from within there, thus insuring requests wont overlap... this will negate the need for the periodical as well but i'd give it a delay like 500ms before calling the parent function again.

eg:

PHP Code:

var runInfinitechecks = function() {
    new Ajax("someurl", {
        onComplete: function() {
            // do something with this.response.text
            
            // repeat after 300ms
            (function() {
                // runInfiniteChecks(); or .pass() or .attempt()
                runInfiniteChecks();
            }).delay(300);
        }
    }).request("action=foo&id=44");
}; 


[MoreEle]

Thank you guys. It was a simple solution indeed =)
Change to "setTimeout(start, 0);" and it did the trick. Tried it on the server and there was no delays or bugs what so ever, even though I used 0ms between the "loops". I also posted a whole page of text into the textfield, and it only took about 500ms to update =D

Dimitar:
Yeah i'm gonna try to optimize the mootools.
And add a onComplete with a nice updating or complete icon.