javascript and a DHTML affect

It's been a while since I worked with JavaScript - after my cert I began learning Perl and have worked with it ever since. I am just trying to get my hand back in and for a start with JS, I have written this what I would have said, was a simple, little script to change a border of a div quite simply. I cannot work out however, just where I'm going wrong. Appreciated of any advice/s开发者_运维技巧uggestion or ideas. TY in advance. Here's my script..

<script type="text/javascript">

var i =0;
var e = document.getElementById("text");

e.style.border = "solid black 5px";
e.style.padding = "5px";

var colors = ["red", "yellow", "blue", "green"];

function changeBorder()
{
    e.border = colors[i];
    i++;
    var timer = setTimeout("changeBorder()", 1000);
}

window.onload=function()
{
    changeBorder();
}

</script>



<div id="text">
   <h1>Hello world</h1>I am Mike!.
</div>

try this: http://jsfiddle.net/maniator/CEsJR/

<script type="text/javascript">
    var i =0;
var colors = ["red", "yellow", "blue", "green"];

function changeBorder(e)
{
    //restarts color counter
    i = i % colors.length;

    e.style.border = colors[i] + " solid 5px"
    i++;

    var timer = setTimeout(function(){changeBorder(e)}, 1000);
}

window.onload=function()
{


    var e = document.getElementById("text");

    e.style.border = "solid black 5px";
    e.style.padding = "5px";

    changeBorder(e);
}

</script>



<div id="text">
   <h1>Hello world</h1>I am Mike!.
</div>

the e variable can only be created on page load

You probably want:

e.style.borderColor = colors[i];

then you'll need to worry about i becoming greater than the length of colors.

Something like:

if (i >= colors.length) i=0;

is probably the way to go.

Possible Problem #1

If this is the order in which everything is run on your page, then e = document.getElementById("text") will never work. You can only get an element by its ID after it has been built by the HTML. Neal's suggestion of only getting the element on page load would work. Or, you could move your script to the bottom of the page.

Possible Problem #2

In your changeBorder function, you apply the new color like so:

e.border = colors[i];

But you probably meant to access e.style.border. And even if you did, my understanding is that border is the shortcut style, wherein you define the width, style, and color all in one rule. You'll probably want to use something like e.style.borderColor.

Possible Problem #3

It doesn't look like you're getting far enough for this to be an issue, but keep in mind that as you cycle through the different colors, eventually your i variable will be bigger than the length of the color array. To make sure that never happens, you could use the modulus operator:

e.style.borderColor = colors[i];
i = (i + 1) % 4;

Bonus

When you're only giving setTimeout a function, with no arguments, then you don't need to use quotes. The following is fine, and doesn't require the eval function:

setTimeout(changeBorder, 1000);

Here's an example. In setTimeout, you can just use a pointer to the function you want to execute. I've included a mechanism to restart the color switch after i = 4.