PDA

View Full Version : What is wrong in this code?



lindm
08-14-2007, 06:43 AM
Found a handy function at The Javascript source and want to modify it for variables (name,range1,range2) in within the function. The script however does not work after my mod. As far as I can see with error searching it has to do with the name variable. Can anyone see what is wrong?

Original working code:



<HEAD>

<SCRIPT LANGUAGE="JavaScript">


function checkAll() {
for (var j = 1; j <= 14; j++) {
box = eval("document.checkboxform.C" + j);
if (box.checked == false) box.checked = true;
}
}


// End -->
</script>
</HEAD>


<BODY>

<center>
<form name=checkboxform>
<input type=checkbox name=C1>
C1<br>
<input type=checkbox name=C2>
C2<br>
<input type=checkbox name=C3>
C3<br>
<input type=checkbox name=C4 checked>C4<br>
<input type=checkbox name=C5 checked>C5<br>
<br>
<br>
<input type=button value="Check All" onClick="checkAll()"><br>
<br>
<input type=button value="Switch All" onClick="switchAll()"><br>
</form>
</center>



Modified code


<HEAD>

<SCRIPT LANGUAGE="JavaScript">


function checkAll(start,stop,name) {
for (var j = start; j <= stop; j++) {
box = eval(name + j);
if (box.checked == false) box.checked = true;
}
}


// End -->
</script>
</HEAD>


<BODY>

<center>
<form name=checkboxform>
<input type=checkbox name=C1>
C1<br>
<input type=checkbox name=C2>
C2<br>
<input type=checkbox name=C3>
C3<br>
<input type=checkbox name=C4 checked>C4<br>
<input type=checkbox name=C5 checked>C5<br>
<br>
<br>
<input type=button value="Check All" onClick="checkAll(1,2,document.checkboxform.C)"><br>
<br>
<br>
</form>
</center>

jscheuer1
08-14-2007, 07:30 AM
There are a lot of things wrong with it, but the deal breaker is that:


document.checkboxform.C

is expected to be a string and you haven't quoted it in your call. Do it like so:


<input type="button" value="Check All" onClick="checkAll(1,2,'document.checkboxform.C')">

Twey
08-14-2007, 07:34 AM
Yuck! Ugly code (http://blogs.msdn.com/ericlippert/archive/2003/11/01/53329.aspx), ugly page (http://validator.w3.org/).
<!DOCTYPE html PUBLIC "-//W3C//DTD HTML 4.01//EN" "http://www.w3.org/TR/html4/strict.dtd">
<html>
<head>
<title>Document Sans Titre</title>
<style type="text/css">
div.checkboxes label {
display: block;
text-align: center;
}
</style>
<script type="text/javascript">
function checkAll(form, start, stop, name) {
var f = ((typeof form === "string" || typeof form === "number") ? document.forms[form] : form).elements;

for(var i = start, e; i < stop; ++i)
if((e = f[name.replace(/&#37;d/g, i)]) && e.checked === false)
e.checked = true;
}
</script>
</head>
<body>
<form action="">
<div class="checkboxes">
<label>
<input type="checkbox" name="C1"> C1
</label>
<label>
<input type="checkbox" name="C2"> C2
</label>
<label>
<input type="checkbox" name="C3"> C3
</label>
<label>
<input type="checkbox" name="C4" checked="checked"> C4
</label>
<label>
<input type="checkbox" name="C5" checked="checked"> C5
</label>
<input type="button" value="Check All" onclick="checkAll(this.form, 1, 2, 'C%d');">
</div>
</form>
</body>
</html>

lindm
08-14-2007, 07:58 AM
Alright working now. Was there anything else seriously bad about the code?

Thanks

Twey
08-14-2007, 08:38 AM
Nothing that I haven't changed. The abuse of <br> was one; lack of a DOCTYPE; lack of a title; lack of a block element inside the <form>; and the use of the deprecated presentational element <center>.

jscheuer1
08-14-2007, 03:40 PM
Nothing that I haven't changed. The abuse of <br> was one; lack of a DOCTYPE; lack of a title; lack of a block element inside the <form>; and the use of the deprecated presentational element <center>.

You (Twey) left out the use of eval(), and the use of the language attribute while omitting the type attribute - possibly more. Basically, before you (lindm) ever modified the code, even though it worked, it was a textbook example of what not to do.

Twey
08-14-2007, 03:53 PM
You (Twey) left out the use of eval()I did, however, link to an article entitled "eval is evil" which I suspected would get across my bearing on the matter.
and the use of the language attribute while omitting the type attribute - possibly more.I linked to the validator too :)
Basically, before you (lindm) ever modified the code, even though it worked, it was a textbook example of what not to do.Agreed. Be very wary of scripts gleaned from sites like this one or Javascript Kit, they tend to be very old and often very far from good coding practices.

lindm
08-15-2007, 03:28 PM
Thank you both!

I now have a second problem, will try to explain it.... The base is a php script that stores the html code in a variable and then writes this to a another html page using fwrite. The problem is that the single quote around the name variable of the javascript (checkAll(1,2,'NAME') is interpreted as not being a string within the php code if I understand it right. Have been looking for solutions but none found. (hope you understood it) I have been looking for a way to create a string in javascript from a variable so that I don't have to have single brackets around the name variable. Example:

Action without quote:
onClick="checkAll(1,2,document.checkboxform.C)"

Javascript:
function checkAll(start,stop,name) {
newname = createstring of name function; //this is my possible solution
for (var j = start; j <= stop; j++) {
box = eval(newname + j);
if (box.checked == false) box.checked = true;
}
}

Do you know of any such solution? Or should I try to rewrite the whole function?

Twey
08-15-2007, 03:48 PM
should I try to rewrite the whole function?I already did it for you. And the page, too.

lindm
08-15-2007, 04:31 PM
I already did it for you. And the page, too.

Yes many thanks, but it still has the citation of the name variable


<input type="button" value="Check All" onclick="checkAll(this.form, 1, 2, 'C%d');">

the 'C%d' to be more specific which causes me problems in my php script. Is there anyway to call the function without citation marks within the input tag?

Twey
08-15-2007, 06:21 PM
You can just escape the quotes:
<input type="button" value="Check All" onclick="checkAll(this.form, 1, 2, \'C&#37;d\');">The best solution, though, is always to drop out of PHP parsing mode when outputting HTML. This makes for neater, more readable code, both client- and server-side.

lindm
08-15-2007, 09:23 PM
Solved it. Thanks!