tags:

views:

121

answers:

3

Hi,

I have the following javascript but I am having issues within using setTimeout, ie:

function setUrl()
{
  // Get system URL details
  var url_path = new String(window.location.pathname);

  var new_url;

  new_url = window.location.host;
  }
  setTimeout("setValue('ONE_UP_URL',new_url);",2000);
}

But for some reason, I am getting the error: 'new_url' is undefined.

Would really appreciate your help on calling this javascript function using setTimeout.

Thanks.

+6  A: 

You have a rogue closing brace. Either there is more code missing, or you just need to remove it. (The line above setTimeout.)

Also, you should replace this:

setTimeout("setValue('ONE_UP_URL',new_url);",2000);

with this:

setTimeout(function() { setValue('ONE_UP_URL', new_url); }, 2000);
John Fisher
+7  A: 

Don't use a string as the first argument of the setTimeout function call, use an anonymous function.

You also had an extra curly brace:

function setUrl() {
  // Get system URL details
  var url_path = window.location.pathname,
      new_url = window.location.host;

  setTimeout(function () {
    setValue('ONE_UP_URL',new_url);
  }, 2000);
}

If you use a string, it will evaluated and that is not really recommended.

Evaluating code by using eval (and its relatives, Function, setTimeout, and setInterval) is considered dangerous, because they will execute the code you pass with the privileges of the caller, and almost all the times there is a workaround to avoid them.

Other minor things:

  • The call to the String constructor in your code is redundant, since window.location.pathname is already a string.
  • You can declare your function variables on a single var statement.
CMS
+1 for the extra details.
ChaosPandion
+1  A: 

Try updating it to something like:

function setValue( s, variable ) {
    alert( s );
    alert( variable );
}

function setUrl()
{
  // Get system URL details
  var url_path = window.location.pathname, 
      new_url = window.location.host;

  setTimeout(
    function() {
        setValue('ONE_UP_URL',new_url);
    }, 2000 );
}
meder
This is the third copy of the correct answer. Shouldn't you delete this?
ChaosPandion
You honestly haven't seen duplicate answers on SO before? And in my solution I actually took the time to write the setValue function to demonstrate, in addition to cleaning up more of the code.
meder
how is multiple declarations on a single line *clearer*? You actually reduced readability.
Jonathan Fingland
When CP initially responded it was two var statements, I just forgot a `\n` - give me a break ;)
meder