Help me code better JS scripts [Archive]

gingerj

12-15-2005, 03:56 PM

Hi, I'm really new to Javascript and the HTML DOM and I produce really awful looking programs can anyone give me some pointers at producing better source.

I've recently made a menu script that has a few div's inside one another, but I'm sure there's better ways of writing it. As right now I repeat alot of source, could anyone show me a better means?

<html>

<head>

<title>Menu Test</title>

<link rel="stylesheet" href="global.css" type="text/css"/>

<script language="javascript">

function hidesearchform() {

var searchform;
var searchform1;
var searchform3;
var searchform4;
searchform = document.getElementById("wholeformcontainer");
searchform1 = document.getElementById("maintestarea");
searchform3 = document.getElementById("shadowlayer");
searchform4 = document.getElementById("shadowlayer2");
searchform.style.visibility="hidden";
searchform1.style.visibility="hidden";
searchform3.style.visibility="hidden";
searchform4.style.visibility="hidden";
}

function expandsearchform () {

var expandform;
var expandform1;
var expandform3;
var expandform4;
expandform = document.getElementById("wholeformcontainer")
expandform1 = document.getElementById("maintestarea");
expandform3 = document.getElementById("shadowlayer");
expandform4 = document.getElementById("shadowlayer2");

if (expandform.style.visibility == "hidden")
{
expandform.style.visibility="visible";
}
else
{
expandform.style.visibility="hidden";
}
if (expandform1.style.visibility == "hidden")
{
expandform1.style.visibility="visible";
}
else
{
expandform1.style.visibility="hidden";
}
if (expandform3.style.visibility == "hidden")
{
expandform3.style.visibility="visible";
}
else
{
expandform3.style.visibility="hidden";
}
if (expandform4.style.visibility == "hidden")
{
expandform4.style.visibility="visible";
}
else
{
expandform4.style.visibility="hidden";
}

}

</script>

</head>

<body OnLoad="hidesearchform()">

<div id="menubutton"><a href="#" OnClick="expandsearchform()">Modify Search</a></div>
<div id="wholeformcontainer">
<div id="maintestarea"></div>
<div id="shadowlayer"></div>
<div id="shadowlayer2"></div>
</div>
<div id="testworkingarea"></div>

</body>

</html>

Thanks in advance!

Twey

12-15-2005, 04:20 PM

Javascript:

1) Indent nicely.
2) Reuse variables.
3) If you find yourself declaring variables in functions over and again, make them global.
4) You needn't repeat "var" all the time; use one "var" keyword and seperate var=value pairs with commas.
5) If you only have one statement in an if or else, you don't need to use braces. Doesn't apply to functions.
6) Don't use tabs. They're too big. I usually use two spaces.
7) Use arrays and for loops rather than long lists of variables where possible.
8) A matter of opinion, but I prefer the "Java" brace layout style (opening braces are always on the same line as their keywords; closing braces go on a seperate line, unless they have a related keyword directly after).

function fish() { // function definition - increase indentation on following lines
if(blather) { // keyword - increase indentation again
some_code();
} else { // end of keyword - decrease indentation on _this_ line and those following - but then a related keyword, so increase again
some_mode_code();
} // end keyword, on a seperate line. Decrease indentation here and onwards.
} // end function definition.
HTML:
1) Indent nicely.
2) Don't use double-linebreaks, unless you want to make something really stand out in the code. Loses its effect if overused.
3) HTML (tags, attributes and attribute values if a set value [like action="get" and shape="rect"]) should be all lower-case. In fact, this is a validation requirement in XHTML.
4) The "language" attribute for <script> is deprecated for "type."
5) The validator (http://validator.w3.org/) is your friend as far as content's concerned, though it won't help with code layout.

Your code, neatened and pruned:

<html>
<head>
<title>Menu Test</title>
<link rel="stylesheet" href="global.css" type="text/css"/>
<script type="text/javascript">
firsttime = true; // avoid defining variables twice.
// we can't have the variables defined until the page loads, because they need their respective elements.
searchforms = new Array();

function init() {
firsttime = false;
searchforms[0] = document.getElementById("wholeformcontainer");
searchforms[1] = document.getElementById("maintestarea");
searchforms[2] = document.getElementById("shadowlayer");
searchforms[3] = document.getElementById("shadowlayer2");
}

function hidesearchform() {
if(firsttime) init(); // put it here, to avoid messing up your onload.
for(var i=0;i<searchforms.length;i++)
searchforms[i].style.visibility = "visible";
}

function expandsearchform () {
for(var i=0;i<searchforms.length;i++)
if(searchforms[i].style.visibility == "hidden")
searchforms[i].style.visibility = "visible";
}
</script>
</head>
<body onload="hidesearchform()">
<div id="menubutton"><a href="#" onclick="expandsearchform()">Modify Search</a></div>
<div id="wholeformcontainer">
<div id="maintestarea"></div>
<div id="shadowlayer"></div>
<div id="shadowlayer2"></div>
</div>
<div id="testworkingarea"></div>
</body>
</html>