Ce:
document.getElementById(selectElements[i].id) != ""
est faux. Vous voulez vérifier si l'élément a un identifiant, donc simplement faire:
selectElements[i].id != ""
Je voudrais souligner que vous pouvez améliorer votre code ici et là:
1 Ne pas écrire for
boucles qui obtiennent la propriété length
pour chaque itération,
faire à la place:
for(i = 0, num = selectElements.length; i < num; i++) {
...
}
Seulement si vous vous attendez selectElements
croître ou Shrin k il serait logique de requery la valeur de la propriété, mais dans ce cas, vous ne devriez probablement pas écrire une boucle for
de toute façon.
2: Ne pas écrire nodelist[index]
Pour nodelists, comme retourné par getElementsByTagName()
vous ne devriez pas écrire nodelist[index]
(même si la plupart de soutien du navigateur). La méthode DOM standard est item
, écrivez donc nodelist.item(index)
à la place.
3 récupérer les éléments du liste qu'une seule fois
Si vous avez besoin d'un élément de la liste plus d'une fois, le stocker dans une variable locale. votre boucle deviendrait:
for(i = 0, selectElement; i < selectElements.length; i++) {
selectElement = selectElements.item(i);
...more code using selectElement...
}
Notez la déclaration de la variable selectElement
dans la boucle. puisque vous ne l'utilisez pas en dehors de la boucle, le déclarer évite tout encombrement et assure que si vous déplacez la boucle, vous déplacez la déclaration.
4. comparaisons les moins chères premiers
Vous avez écrit:
selectElement.className == "jumpmenu" &&
selectElement.id != ""
cela pourrait être légèrement améliorée si vous inversez les jambes:
selectElement.id != "" &&
selectElement.className == "jumpmenu"
Ce sera plus rapide, car il est moins Travailler pour vérifier si une chaîne est vide, et pour les cas où la chaîne est vide, nous ne vérifierons même pas le className
5 Ne pas utiliser document.getElementById()
si vous avez déjà l'élément
intérieur de la boucle que vous avez ceci:
jumpmenu = document.getElementById(selectElements[i].id);
Vous obtenez essentiellement l'id de la selectElement
et l'utiliser pour interroger le document pour trouver ....l'élément ayant et id
égal à celui du courant selectElement
. Becuase id
sont uniques dans le document (ou devraient être), vous êtes fondamentalement en train d'écrire une phrase complètement inutile. jumpmenu
et selectElement
se réfèrent à un même objet.
6. Améliorations du gestionnaire de onchange
dans la boucle que vous attribuez un gestionnaire onchange. vous le faites en créant une nouvelle fonction pour chaque itération de boucle. Ceci est le code du gestionnaire:
function() {
if(this.options[this.selectedIndex].value != '') {
// Redirect
location.href=this.options[this.selectedIndex].value;
}
}
Trois choses sont à noter ici: Tout d'abord, le code du gestionnaire onchange contient ce location.href = ...
qui devrait probablement être document.location.href = ...
.
Deuxièmement, vous faites référence deux fois à this.options[this.selectedIndex].value
. Encore une fois, mettez ceci dans une variable locale. Troisièmement, le this
fait référence à l'élément qui a rencontré l'événement onchange au moment où cette fonction est exécutée. Autre que this
et les propriétés de this
, il n'y a pas de variables dans ce gestionnaire qui proviennent de la boucle ou de la fonction initJumpMenus
externe. Vous devez simplement créer une fois, en dehors de la boucle, et l'assigner à chaque itération:
var onchange_handler = function() {
if(this.options[this.selectedIndex].value != "") {
// Redirect
location.href=this.options[this.selectedIndex].value;
}
}
for (...) {
if (...) {
selectElement.onchange = onchange_handler;
}
}
7 Résumé
Mettre tout, c'est toghether comment j'écrire:
function initJumpMenus() {
var handler = function() {
var value = this.options.item(this.selectedIndex).value;
if(value != "") {
// Redirect
document.location.href = value;
}
}
var selectElements = document.getElementsByTagName("select");
for(var i = 0, num = selectElements.length, selectElement; i < num; i++) {
selectElement = selectElements.item(i);
// Check for the class and make sure the element has an ID
if(selectElement.id != "" &&
selectElement.className == "jumpmenu"
) {
selectElement.onchange = handler;
}
}
}
N'utilisez pas les «menus de saut» '