1) I'd start with a better HTML template, you forgot the DOCTYPE and meta tags:
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8">
<title></title>
</head>
<body>
</body>
</html>
2) Don't use topalign
and align
HTML attributes. Keep the styles and layout separate, use CSS:
body {
margin: 15px 0;
text-align: center;
}
3) In JavaScript it is ussually preferred to put braces on the same line, because the ASI (automatic semicolon insertion) feature can be problematic, consider this buggy example:
return
{
foo: 'foo'
}
JavaScript will do this:
return; //<--!!!!
{
foo: 'foo'
}
See the semicolon it added? Now you return undefined
, and not an object.
Now, you forgot a bunch of semicolons. Again, ASI can fill them up for you, but I don't recommend you rely on it if you're a beginner, so my advice is always put the semicolons.
5) Check your code in JSHint, this is what I got:
13 warnings
1 Missing semicolon.
5 Missing semicolon.
6 document.write can be a form of eval.
6 Missing semicolon.
8 Missing semicolon.
9 document.write can be a form of eval.
9 Missing semicolon.
13 Missing semicolon.
14 document.write can be a form of eval.
14 Missing semicolon.
16 Missing semicolon.
17 document.write can be a form of eval.
17 Missing semicolon.
One undefined variable
5 randomnumber
6 randomnumber
13 randomnumber
14 randomnumber
One unused variable
1 time
Go one by one, try to fix all those warnings.
6) Avoid document.write
, see Why is document.write considered bad practice. In other words, use the DOM.
7) Inline events are bad practice, again, use the DOM, with addEventListener
.
8) Using the DOM can be expensive. Try to always add elements all at once. Document fragments help you improve performance, because you don't mess with the DOM on every iteration of the loop.
9) Finally, separate concerns; the single responsibility idea. This is how I would write it taking all those points into consideration:
// Use the DOM to create elements
function makeInput() {
var input = document.createElement('input');
input.type = 'checkbox';
return input;
}
function append(ele, parent) {
(parent||document.body).appendChild(ele);
}
function makeLights() {
var i, j;
var br = document.createElement('br');
// Use a document frag to improve performance
var frag = document.createDocumentFragment();
// No need to reset "j" on every iteration
for (i=0; i<=32; i+=2) {
for (j=0; j<=i; j++) append(makeInput(), frag);
append(br.cloneNode(), frag);
}
for (i=30; i>=0; i-=2) {
for (j=0; j<=i; j++) append(makeInput(), frag);
append(br.cloneNode(), frag);
}
// Append to document all at once
append(frag);
}
function randomColor() {
return Math.floor(Math.random() * 2323232);
}
// Use event delegation for best performance
document.body.addEventListener('mouseover', function(e) {
if (e.target.tagName == 'INPUT') {
document.bgColor = randomColor();
}
});
makeLights(); // init
Demo: http://jsbin.com/yiyic/1