Discussion :

[ProgElecT]

Salut

Dans un code qui fonctionne, je voudrais optimiser le remplissage de 2 <select>, id="indicatif1" et id="indicatif2".
Dans ma version de base j'ai 2 boucles for.

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
				for (i = 0; i < dataOM.length; i++) {
					z = document.createElement("option");
					z.setAttribute("value", dataOM[i].indicatif);
					z.setAttribute("id", "choix" + i);
					t = document.createTextNode(dataOM[i].indicatif);
					z.appendChild(t);
					document.getElementById("indicatif1").appendChild(z);
					if (dataOM[i].indicatif == nomindicatif){numidx=i;}
				}
				document.getElementById("nbr").innerHTML = "Listage " + dataOM.length + " indicatifs";
				document.getElementById("indicatif1").selectedIndex = numidx;
				affiche("indicatif1");
				for (i = 0; i < dataOM.length; i++) {
					z = document.createElement("option");
					z.setAttribute("value", dataOM[i].indicatif);
					z.setAttribute("id", "choix" + i);
					t = document.createTextNode(dataOM[i].indicatif);
					z.appendChild(t);
					document.getElementById("indicatif2").appendChild(z);
				}

Mes 2 select se remplissent correctement.

Par contre si je modifie de cette façon

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
				for (i = 0; i < dataOM.length; i++) {
					z = document.createElement("option");
					z.setAttribute("value", dataOM[i].indicatif);
					z.setAttribute("id", "choix" + i);
					t = document.createTextNode(dataOM[i].indicatif);
					z.appendChild(t);
					document.getElementById("indicatif1").appendChild(z);
					document.getElementById("indicatif2").appendChild(z);
					if (dataOM[i].indicatif == nomindicatif){numidx=i;}
				}

Ben là , je n'ai que le <select> id="indicatif2" qui est rempli.
Je suis super déçu, je croyais, depuis maintenant 1 an et demi que je me suis mis au JavaScript, avoir compris le minimum des choses, ben non apparemment.

Une petite explication s'il vous plait

[psychadelic]

bon, puisque dans le titre il est question d'optimisation, alors vite fait au moins 2 ou 3 choses:
for (i = 0; i < dataOM.length; i++) à chaque itération l'interprétreur JS en passe par une évaluation de cette valeur, ce qui fait perdre du temps, c'est minime, mais les petits ruisseaux font de grandes rivières.
Donc préférer écrire :
for (let i = 0, iMax =dataOM.length ; i < iMax ; i++)au passage, l'ajout du let pour les 2 variables (i et iMax) permet de s’assurer qu'il n'y aura pas de pb de collusion sur la portée d'un i ou d'un iMax ailleurs

même idée pour document.getElementById("indicatif1").appendChild(z); inscrit à l'intérieur de la boucle : l'interpréteur JS doit aussi réévaluer cette valeur à jaque tour de boucle, ce qui prend un peu plus de temps encore que le "truc" précédent.

déclarer ainsi:

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
 
var Indicatif_1 = document.getElementById("indicatif1");
//...
for (let i = 0, iMax =dataOM.length ; i < iMax ; i++) {
//..
  Indicatif_1.appendChild(z);

Pour ton bugg j'ai pas regardé, peut-être plus tard, si personne ne t'a répondu d'ici la

[ProgElecT]

Ok, merci de ces informations, c'est déjà sa de pris et d'appris.

[tsuji]

En ce qui concerne l'optimisation, l'opération qui coûte un peu plus est getElementById() et on peut facilement diminuer la fréquence de la faire appeler. Et puis, les autres, c'est minime.

En ce qui concerne fonctionnel, le problème est de réaliser que z est une référence d'un objet. Où se trouve-il finalement ? La dernière opération de le placer. Pour corriger, on crée un clone, clone dit profond, d'elle et place le clone dans indicatif2.

En somme, comme ça.

Code javascript :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
                                var indicatif1=document.getElementById("indicatif1");
                                var indicatif2=document.getElementById("indicatif2");
				for (var i = 0; i < dataOM.length; i++) {
					var z = document.createElement("option");
					z.setAttribute("value", dataOM[i].indicatif);
					z.setAttribute("id", "choix" + i);
					var t = document.createTextNode(dataOM[i].indicatif);
					z.appendChild(t);
 
					indicatif1.appendChild(z);
					indicatif2.appendChild(z.cloneNode(true));
 
					if (dataOM[i].indicatif == nomindicatif){numidx=i;}
				}

[ProgElecT]

Merci tsuji, c'est impeccable, cela fonctionne bien sûr.

Merci aussi à psychadelic, à vous deux je fini l'année un peu moins bête.

En attendant je vais regarder cette hstoire de .cloneNode pour en savoir plus.

Bonne année à chacun et aux lecteurs de cette discussion.

[psychadelic]

effectivement le Bugg est bien celui la, 1 element crée reste unique, et il ne peut pas être à 2 endroits à la fois.

Il y a eu tout un débat sémantique ici (je ne retrouve pas ou) sur la signification / traduction du mot anglais "append", dont pour moi la meilleure traduction possible est le terme "advenir" ( il doit y avoir du Latin genre "faire venir" ).

Toujours est il que la doc MDN est tres claire sur le sujet : https://developer.mozilla.org/fr/doc...de/appendChild

Cela signifie qu'un noeud ne peut pas être à deux points du document simultanément. Donc, si le nœud a déjà un parent, le nœud est d'abord retiré, puis ajouté à la nouvelle position.

[EDIT] et bonne année à vous aussi, ainsi qu'a tous ceux qui sont passé par la

[tsuji]

Bonne année à vous aussi, ProgElecT; et à tout le monde !

[CosmoKnacki]

Tu peux aussi limiter les modifications de l'arbre DOM en utilisant un fragment. De cette manière, tu n'effectues plus que deux appendChild sur tes selects (l'un avec clonage profond du fragment, l'autre non) en dehors de la boucle.

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
let fragment = document.createDocumentFragment();
 
for (let i = 0, iMax = dataOM.length; i < iMax; i++) {
    let option = document.createElement('option');
    option.setAttribute('value', dataOM[i].indicatif);
    option.appendChild(document.createTextNode(dataOM[i].indicatif));
    fragment.appendChild(option);
    if ( dataOM[i].indicatif == nomindicatif ) numidx = i;
}
 
document.getElementById('indicatif1').appendChild(fragment.cloneNode(true));
document.getElementById('indicatif2').appendChild(fragment);

À noter que je n'ai volontairement pas mis l'attribut id sur les éléments options. En effet, l'attribut id doit être unique dans un document html. Donc tu devras trouver comment t'en passer ou utiliser un autre attribut.

[ProgElecT]

Salut

Merci CosmoKnacki, c'est Ok,

À noter que je n'ai volontairement pas mis l'attribut id sur les éléments options. En effet, l'attribut id doit être unique dans un document html. Donc tu devras trouver comment t'en passer ou utiliser un autre attribut.

Tout compte fait je viens de m'apercevoir que n'ai pas besoin de cette information, les index de mon tableau dataOM correspondant forcement au selectedIndex de mes deux éléments Dom <select>.
Je récupère donc cette indice dans ma fonction de cette façon let idx = Number(document.getElementById(idSrc).selectedIndex); idSrc étant l'id du <select> utilisé.
Ce qui me permet de parcourir l'ensemble de mon tableau objet (dataOM[idx].nomprenom , dataOM[idx].adresse , dataOM[idx].cp .... ect )
Mon tableau se présentant comme suit,

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
let dataOM = [
  {
    "numid": 0,
    "indicatif": "F0FYF",
    "nomprenom": "BUSSAT Jean-francois",
    "adresse": "379 CHEMIN DE LA PAREUSAZ",
    "cp": "74370",
    "ville": "VILLAZ",
    "infos": "",
    "position": {
        "lat": 45.9410452,
        "lng": 6.1843334,
        "alt": 642
    }
  },
  {
    "numid": 1,
    "indicatif": "F1ACF",
    "nomprenom": "MORARAU Claude",
    "adresse": "409 CHEMIN DE CUGNON",
    "cp": "74170",
    "ville": "LES CONTAMINES MONTJOIE",
    "infos": "",
    "position": {
        "lat": 45.812625499999996,
        "lng": 6.729692,
        "alt": 1165
    }
  },
  {
    "numid": 2,
    "indicatif": "F1ADG",
// .......

Vous pouvez voir un aperçu ICI, pas de Ctrl U pour voir le code, vous seriez sûrement déçu de la rédaction du code (je suis toujours débutant).

[Beginner.]

Salut,

Récemment on a eu le même problème ici : #3...

Vous pouvez voir un aperçu ICI, pas de Ctrl U pour voir le code, vous seriez sûrement déçu de la rédaction du code (je suis toujours débutant).

Merci. Et si c'est un autre débutant qui regarde le code, ça va ?

EDIT : Ah le calcul de distance entre deux points de la carte ça m'intéresse, j'y connais rien pour l’instant mais j'y reviendrais peut-être un jour...

[SpaceFrog]

Juste un petit complément pour te filer ce petit tuyau:
Lorsque je dois créer des éléments en boucle, je privilégie en général la méthode de clone qui est plus rapide.
Je créée l'élément "template" avant la boucle, je le clone et le complète dans la boucle.

[NoSmoking]

Bonjour,
faut-il parler de simplification ou d'optimisation de boucle !?!

Dans le cas présent je parlerais plutôt de simplification en vue d'une meilleure lisibilité plutôt que d'optimisation.

L'optimisation serait inintéressante dans la mesure où cette partie du code serait répétée plusieurs fois et contribuerait à faire « ramer », ce qui n'est pas le cas car appelée en depuis et ce une seule fois.

Les solutions proposées, travail dans un DocumentFragment, mise en cache des valeurs/variables, clonage ... etc sont toutes à fait pertinentes et ne doivent pas être écartées.

Cependant lorsque l'on sait qu'il existe un constructor Option() je trouve dommage de ne pas l'utiliser, certains diront qu'il est plus lent que le traditionnel createElement mais une fois de plus dans le cas présent pas sûr de l'impact, d'autant qu'il fait tout en même temps

On pourrait se retrouver avec une boucle « légére » et plus « lisible » du style

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
data.forEach(function(elem, ind) {
    opt = new Option(elem.indicatif, elem.indicatif);
    oSelect_1.add( opt);  
    oSelect_2.add( opt.cloneNode(true));   // un clone ou un nouveau new
});
// valeur par défaut, le test est sorti de la boucle
oSelect_1.value = nomindicatif || data[0].indicatif;

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
data.forEach(function(elem, ind) {
    opt = new Option(elem.indicatif, elem.indicatif);
    oSelect_1.options[ind] = opt;
    oSelect_2.options[ind] = opt.cloneNode(true);
});
// valeur par défaut, le test est sorti de la boucle
oSelect_1.value = nomindicatif || data[0].indicatif;

le choix des armes est assez important, mais lequel fera VRAIMENT la différence dans ton cas !??!

Concernant le clonage l'exemple de CosmoKnacki est tout à fait judicieux dans le sens où il n'y a pas un clonage pour chaque <option> mais un clonage global.

Tu pourrais également créer totalement tes deux <select> et cela me semblerait également aller plus dans le sens d'une simplification
par exemple :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
4
5
6
7
8
9
10
11
12
13
var data = dataOM;
var oDest = document.getElementById("zone-select");
// création select 
var oSelect = document.createElement("SELECT");
data.forEach(function(elem) {
  // ajout direct des options
  oSelect.add(new Option(elem.indicatif, elem.indicatif));
});
// ajout au parent et affectation ID
oDest.appendChild(oSelect).id = "indicatif1";
oDest.appendChild(oSelect.cloneNode(true)).id = "indicatif2";
// valeur par défaut, le test est sorti de la boucle
oSelect.value = nomindicatif || data[0].indicatif;

il te faudrait également revoir d'autres éléments de ton code, comme l'affectation des événement par exemple, mais c'est pour l'esprit.

Voilà quelques éléments de réflexion supplémentaires

[ProgElecT]

Salut

Bonjour,
...Voilà quelques éléments de réflexion supplémentaires

, je voulais juste éviter 2 boucles for .... ben voilà que je vais devoir ingurgiter toutes ses informations.

Je suis donc occupé pour un moment, heureusement que l'on est pas à la bonne saison, en ce moment je ne bricole pas autour de la maison j'ai donc du temps.

Merci à tous pour le boulot ces informations précieuses .