[thelist] help me compress this JS function?

[Jeff Howden]

tom,

><><><><><><><><><><><><><><><><><><><><><><><><><><><><><
> From: Tom Dell'Aringa
>
> I'm thinking I can slightly improve this function, but
> I'm not sure where. Here is the function, which takes a
> group of radio buttons and either selects them all or
> deselects them all, while changing the name of the
> controlling button to reflect the method:
><><><><><><><><><><><><><><><><><><><><><><><><><><><><><

oooh, optimizing is fun.

><><><><><><><><><><><><><><><><><><><><><><><><><><><><><
> function toggleAllCheckboxes(oForm, cbGroup, button)
> {
> 	var len = oForm.length;
> 	for(i=0;i<len;i++)
> 	{
> 		currentElement = oForm.elements[i];
> 		if(oForm.elements[i].type == "checkbox")
> 		{
> 			var matchGroup =
> currentElement.value.substring(0,4);
> 			if(matchGroup == cbGroup)
> 			{
> 				if(currentElement.checked == true)
> 				{
> 					currentElement.checked = false;
> 					button.value = "Select All";
> 				}
> 				else
> 				{
> 					currentElement.checked = true;
> 					button.value = "Deselect All";
> 				}
> 			}
> 		}
> 	}
> }
><><><><><><><><><><><><><><><><><><><><><><><><><><><><><

part of the problem with your current function is that the button's value is
set for each iteration through the loop.  you really should only be setting
it once.

also, it seems, based on your function that you're toggling whether the
checkboxes are checked, and not actually setting all to checked or all to
unchecked.  this is evidenced by the fact that you look for the checked
property value inside the loop prior to setting it to the opposite.  so,
suppose you had the following group of checkboxes (1's represent checked,
0's represent unchecked):

1
0
1
1
1
0
1

calling your function on this group of checkboxes would result in:

0
1
0
0
0
1
0

><><><><><><><><><><><><><><><><><><><><><><><><><><><><><
> if(matchGroup == cbGroup) ? uncheck boxes : check boxes
><><><><><><><><><><><><><><><><><><><><><><><><><><><><><

there's no if.  the tertiary operator is used as a statement.

myElement.checked = (matchGroup == cbGroup) ? true : false;

or, since the statement (matchGroup == cgGroup) will return a boolean
already, drop the tertiary operator and reduce it to:

myElement.checked = (matchGroup == cbGroup)

i suspect part of the difficulty with your function is that you're not
naming groups of checkboxes with the same name.  if that's the case, change
that.  in order to operate on groups of checkboxes in an efficient manner,
you really need to have them all share the same name.

here's an example i put together alittle while ago:

http://evolt.jeffhowden.com/jeff/code/checkbox_check_all.cfm

later,

.jeff

http://evolt.org/

NOTICE:  members.evolt.org web and email address are changing!
---------------------------------------------------------------------
| OLD:                            | NEW:                            |
| jeff at members.evolt.org          | evolt at jeffhowden.com            |
| http://members.evolt.org/jeff/  | http://evolt.jeffhowden.com/    |
---------------------------------------------------------------------