Conseils pour optimisatisation

**Leben75** · 25/06/2012, 16h17

Bonjour,

Je ne viens pas pour "crier au secours" mais pour crier: conseillez moi !

Voilà, dans le cadre d'un développement, j'ai écris pour la première fois des fonction en JS presque tout seul.
Etant débutant, j'aimerai avoir des conseils, critiques pour optimiser mon code.
Sa m'aidera à l'alléger, "l'accélérer" et surtout sa m'aidera à avancer !

Je remercie d'avance ceux qui se lancerons dans cette lecture, "décortication" de ce code:

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
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
 
var langue = document.documentElement.lang; /* Récupère la langue: balise html (var global) */
var save_design; /* sauvegarde du design (var global) */
 
 
function loadMain(){ /* fonction utilisé dans la balise <body onLoad=""> */
    design(save_design);
}
 
 
function getXMLHttpRequest() { /* Instance XMLHttpRequest */
    var xmlhttp = null;
    if (window.XMLHttpRequest || window.ActiveXObject) {
        if (window.ActiveXObject) {
            try {
                xmlhttp = new ActiveXObject("Msxml2.XMLHTTP");
            } 
            catch(e) {
                xmlhttp = new ActiveXObject("Microsoft.XMLHTTP");
            }
        }
        else {
            xmlhttp = new XMLHttpRequest(); 
        }
    } 
    else {
        alert("Votre navigateur ne supporte pas l'objet XMLHTTPRequest.");
        return null;
    }
    return xmlhttp;
}
 
 
function getRSS(){ /* obtient un lien rss via un fichier .txt en fonction de la langue reconnu(celle du <html lang> */
    var rss = getXMLHttpRequest();
    rss.open('GET', 'para.txt', false);
    rss.send(null);
    var ligne = rss.responseText.split(/\n/g);
    switch(langue){
        case 'fr':
            linkRss = ligne[1];
            return linkRss;
        break;
        case 'en':
            linkRss = ligne[2];
            return linkRss;
        break;
        case 'de':
            linkRss = ligne[3];
            return linkRss;
        break;
        case 'es':
            linkRss = ligne[4];
            linkRss;
        break;
        case 'it':
            linkRss = ligne[5];
            return linkRss;
        break;
    }
}
 
 
function showNews(){
    getRSS(); /* obtient le lien rss */
    var New = getXMLHttpRequest(); /* instance XMLHttpRequest */
    New.onreadystatechange = function() {
        if(New.readyState == 4){
            if(New.status == 200){
                xmlDoc=New.responseXML;
                var x=xmlDoc.getElementsByTagName('item');
                for(var i=0; i<x.length; i++){
		    document.write('<div class="info">');
	            document.write('<h5 class="title">');
		    document.write(x[i].getElementsByTagName('title')[0].childNodes[0].nodeValue);
		    document.write('</h5> <br />');
		    document.write('<p>');
		    document.write(x[i].getElementsByTagName('description')[0].childNodes[0].nodeValue);
		    document.write('</p> <br />');
		    document.write('<a href=');
		    document.write(x[i].getElementsByTagName('link')[0].childNodes[0].nodeValue);
		    document.write('> Get more...</a> <br /> <br />');
		    document.write('<p class="publie">');
		    document.write(x[i].getElementsByTagName('pubDate')[0].childNodes[0].nodeValue);
		    document.write('</p>');
		    document.write('</div> <br />');
		}
            }
	    else {
		document.write('<p style="margin-left: 55px"> Ethernet Offline </p>');
	    }
	}
    }
    New.open('GET', linkRss, false);
    New.send(null);
}
 
 
function design(style) { /* change le design sur un click d'un input de type select*/
    if (style == 'eurosatory') {
        save_design = 'eurosatory';
        var lien_css = document.createElement('link');
        lien_css.href = "../css/design-eurosatory.css";
        lien_css.rel = "stylesheet";
        lien_css.type = "text/css";
        document.getElementsByTagName("head")[0].appendChild(lien_css);
    }
    else if (style == 'blue') {
        save_design = 'blue';
        var lien_css = document.createElement('link');
        lien_css.href = "../css/design-blue.css";
        lien_css.rel = "stylesheet";
        lien_css.type = "text/css";
        document.getElementsByTagName("head")[0].appendChild(lien_css);	
    }
    else if (style == 'pulse') {
        save_design = 'pulse';
        var lien_css = document.createElement('link');
        lien_css.href = "../css/design-pulse.css";
        lien_css.rel = "stylesheet";
        lien_css.type = "text/css";
        document.getElementsByTagName("head")[0].appendChild(lien_css);	
    }
    else {
        save_design = 'origine';
        var lien_css = document.createElement('link');
        lien_css.href = "../css/design-origine.css";
        lien_css.rel = "stylesheet";
        lien_css.type = "text/css";
        document.getElementsByTagName("head")[0].appendChild(lien_css);
    }
}

**PP(Team)** · 25/06/2012, 17h23

Bonjour,

Voici mes retours pour ton code :
- fonction "getRSS" : pourquoi "rss.send(null)" ?
- fonction "getRSS" : ton switch ne renvoie pas toujours une valeur (case "es") ?
- fonction "showNews" : évite les noms de variables ayant des noms "réservé" comme new, case, etc. (tu as une variable qui se nomme "New")
- ligne 65 : fonction "getRSS()" est appelée sans récupérer la valeur alors qu'elle a des "return"...
- ligne 102, 110, 118, 126 : "var lien_css = document.createElement('link');". Pourquoi ne pas avoir créé cette variable à la ligne 100 (avant le 1er "if"), cela t'aurait fait gagner 3 lignes de codes.

Voili voilou pour moi.

En espérant que cela puisse t'aider.

Cordialement.

**Watilin** · 25/06/2012, 18h08

Salut,

même cause, même punition : document.write

Là tu appelles document.write au sein de la fonction de rappel d'une requête AJAX. Tu fonces tête baissée dans le

en faisant comme ça : il y a de fortes chances que le flux de la page soit fermé depuis longtemps au moment où la réponse AJAX va arriver, par conséquent ta fonction va rouvrir ce flux, ce qui aura pour effet d'effacer entièrement la page ! On le dit partout mais document.write a la vie dure… C'est une relique du HTML des années 90, il faut l'oublier !

Utilise à la place un innerHTML, c'est déjà mieux. Mais le top du top, apprends à manier le DOM avec createElement, appendChild et toute la clique ! C'est la solution optimale car les navigateurs actuels sont capables de préparer des fragments de DOM pendant la phase de pré-compilation du script.
D'accord, certains vont dire, innerHTML est lui-aussi bien optimisé aujourd'hui, mais il nécessite une analyse de chaîne en plus, du coup il n'a pas ma préférence.

Voilà un exemple de construction du DOM :

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
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
			if (New.status == 200) {
				xmlDoc = New.responseXML;
				var x = xmlDoc.getElementsByTagName('item');
				for (var i = 0; i < x.length; i++) {
					// document.write('<div class="info">');
					var $div = document.createElement("div");
					$div.className = "info";
 
					//document.write('<h5 class="title">');
					var $h5 = document.createElement("h5");
					$h5.className = "title";
					$h5.appendChild(document.createTextNode(
						x[i].getElementsByTagName('title')[0].firstChild.nodeValue));
					$div.appendChild($h5);
 
					// document.write('</h5> <br />');
 
					// document.write('<p>');
					var $p = document.createElement("p");
					$p.appendChild(document.createTextNode(
						x[i].getElementsByTagName('description')[0].firstChild.nodeValue));
					$div.appendChild($p);
 
					// document.write('</p> <br />');
 
					// document.write('<a href=');
					var $a = document.createElement("a");
					$a.href = x[i].getElementsByTagName('link')[0].firstChild.nodeValue;
					$a.appendChild(document.createTextNode('Get more...'));
					$div.appendChild($a);
 
					// document.write('<p class="publie">');
					var $pPublie = document.createElement("p");
					$pPublie.className = "publie";
					$pPublie.appendChild(document.createTextNode(
						x[i].getElementsByTagName('pubDate')[0].firstChild.nodeValue));
					$div.appendChild($pPublie);
 
					// document.write('</div> <br />');
					document.body.appendChild($div);
				}
			} else {
				// document.write('<p style="margin-left: 55px"> Ethernet Offline </p>');
				var $p = document.createElement("p");
				$p.style.marginLeft = "55px";
				$p.appendChild(document.createTextNode("Ethernet offline"));
				document.body.appendChild($p);
			}

J'ai volontairement oublié les <br />, personnellement je trouve qu'ils ne servent à rien : on peut toujours rajouter des marges avec CSS.

Pour la fonction getRSS, juste mon point de vue personnel : faut arrêter de délirer avec l'AJAX synchrone. L'AJAX a été inventé pour être asynchrone, et permettre à l'utilisateur de continuer à interagir avec la page pendant que la requête arrive… Autrement, il ne sert absolument à rien ! Retire-moi donc ce false et traite la réponse dans une fonction de rappel.
Sinon, petite astuce :

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
/*
	switch (langue) {
	case 'fr':
		linkRss = ligne[1];
		return linkRss;
		break;
	case 'en':
		linkRss = ligne[2];
		return linkRss;
		break;
	case 'de':
		linkRss = ligne[3];
		return linkRss;
		break;
	case 'es':
		linkRss = ligne[4];
		linkRss;
		break;
	case 'it':
		linkRss = ligne[5];
		return linkRss;
		break;
	}
	*/
	var codes = {
		"fr": 1,
		"en": 2,
		"de": 3,
		"es": 4,
		"it": 5
	};
	linkRss = ligne[codes[langue]];
	return linkRss;

Même genre d'astuce avec la fonction design, tu n'as pas besoin de tous ces if/else, il suffit de traiter le paramètre style directement :

Code :

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

1
2
3
4
5
6
7
8
function design(style) { /* change le design sur un click d'un input de type select*/
	save_design = style;
	var lien_css = document.createElement('link');
	lien_css.href = "../css/design-" + style + ".css";
	lien_css.rel = "stylesheet";
	lien_css.type = "text/css";
	document.getElementsByTagName("head")[0].appendChild(lien_css);
}

**Leben75** · 26/06/2012, 11h36

Bonjour,

Je vous remercie de vos conseils. Notamment Waitlin pour m'avoir proposé ses astuces. Court et simple.

Juste, l'exemple ddu fonctionnement du DOM n'est pas opérationnel.
Je me suis pas encore penché sur le DOM et sa clique d'appendChild.
Je vais le faire.

Merci !

**Watilin** · 26/06/2012, 14h51

Pour l'exemple du DOM, j'ajoute les nouveaux éléments directement sur le <body>, c'est peut-être ça qui te fait penser que ça ne marche pas. C'est censé apparaître tout en bas de la page. À toi d’adapter ça à ta propre structure HTML !

**Leben75** · 28/06/2012, 15h33

Ah oui, après études de DOM j'ai trouvé, compris comment toutes ces "structures" fonctionnent.

Encore merci !

Conseils pour optimisatisation

JavaScript

Vue hybride

Discussions similaires

Partager

Partager