est-ce que ce code peut-être amélioré ? [important donnée depuis fichier JSON]

leymiris · 09/09/2011, 20h45

Bonjours à tous,

Dans le cadre d'un projet pour lequel je travail, je suis en ce moment entrain de travailler sur l'intérface d'administration. Les données chargé proviennes d'un fichier JSON (format imposé) et j'ai fait pour commencer ce petit bout de code qui permet d'afficher une première liste (collections) et en cliquant sur une des collections je charges une seconde liste (oeuvres). A cette étape tout fonctionne parfaitement mais étant débutant avec JSON j'aurais souhaiter votre aide pour voir si le code pouvait être amélioré.

Je vous remercie grandement d'avance de vos avis et conseils

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
 
/*
 * Script pour affichage des informations du fichier JSON
 */
 
jQuery(document).ready(function($) {
 
    function div_data(element,id, thumbnail, title, text){
        var div  = '<div id="'+element+'-id-'+id+'" class="element '+element+' click">';
            div += '<span class="thumb-profil"><img src="'+thumbnail+'"alt="'+title+'" /></span>';
            div +=  '<h3><a href="#">'+title+'</a></h3>';
            div +=  '<span class="element-text">'+text+'</span>';
            div += '</div>';
 
        return div;
    }
 
    $.getJSON('http://localhost:8888/monde/media/json/collections.json', function(data) {
 
        $.each(data.collections, function(i,data){
 
            //insertion du total de collection dans l'entête
            $("#total-collections").html("("+(data.total)+")");
 
            //DIV des aperçu de collection et oeuvres
 
            var div_collection = div_data("collection", data.id, data.thumbnail, data.title, data.description)
 
            //ajout à la fin du div collection le div_data avec une boucle each
            $(div_collection).hide().appendTo(".collections").fadeIn();
 
        });
 
        $(".element").click(function(){
 
            //récupération de l'ID de la collection lors du clique
            var $id = $(this).attr("id").match(/[0-9+]*$/gi)[0];
 
            //on vide à chaque click le contenu du div artworks et la valeur total
            $("div").remove(".artwork");
            $("#total-artwork").html("");
 
 
            $.each(data.collections, function(i,data){   
 
                //permet de filtrer en fonction de l'ID de la collection cliqué et si elle contient des oeuvres
                if(data.id == $id && data.total != 0){
 
 
                    $("#total-artwork").html("("+(data.total)+")");
 
                    //affichage des oeuvres de la collection choisie
                    $.each(data.arts, function(i,data){ 
 
                        //DIV des aperçus de collection et oeuvres
                        var div_artwork = div_data("artwork", data.id, data.thumbnail, data.title, data.author)
 
                        $(div_artwork).hide().appendTo(".artworks").fadeIn();
 
                    });
 
                }
 
            });
 
        });
 
    });
 
 
 
    //---- Script de debug pour la lecture du fichier json -------//
    $.ajaxSetup({
        "error":function(XMLHttpRequest,textStatus, errorThrown) {   
            alert(textStatus);
            alert(errorThrown);
            alert(XMLHttpRequest.responseText);
        }
    });
 
 
});

galerien69 · 10/09/2011, 10h15

salut,

On pourrait enlever la dupplication de code : tu fais deux fois un foreach qui sont très similaires (l'un pour collections, l'autre pour artwork)
donc genre une fonction qui fait la même chose, mais pour les deux

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
 
function updateList(listType, data, filter){
//listType : collections or artwork
//data : list of collections ou artwork
//filter : ajoute le resultat si return true
 
  //appeler data : element ou obj ou qqch dans le genre serait mieux
  $.each(data.collections, function(id,data){
    if(!filter || filter(data)){
      //insertion du total dans l'entête
      $('#total-'+listType).html('('+(data.total)+')');
 
      //des aperçu de collection ou oeuvres
      var div = div_data(listType, data)
 
      //ajout du div
      $(div).hide().appendTo("."+listType).fadeIn();
    }
  });
}
$.getJSON('http://localhost:8888/monde/media/json/collections.json', function(data) {
  updateList('artwork', data.collections);
  $(".element").click(function(){
    //récupération de l'ID de la collection lors du clique
    var $id = $(this).attr("id").match(/[0-9+]*$/gi)[0];
 
    //on vide à chaque click le contenu du div artworks et la valeur total
    $("div").remove(".artwork");//ici mettre un id plutot qu'un selecteur global
    $("#total-artwork").html("");
 
    updateList('artwork', data.collections, function filterId(data){
      return (data.id == $id && data.total != 0);
    });
  });
 
});

Enfin, on peut éviter d'avoir 50 argument dans l'appele de div_data :

Code :

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

1
2
3
4
5
6
7
8
9
function div_data(elementType,element){
  var div  = '<div id="'+elementType+'-id-'+element['id']+'" class="element '+elementType+' click">';
      div += '<span class="thumb_profil"><img src="'+element['thumbnail']+'"alt="'+element['title']+'" /></span>';
      div +=  '<h3><a href="#">'+element['title']+'</a></h3>';
      div +=  '<span class="element-text">'+element['description']+'</span>';
      div += '</div>';
 
  return div;
}

A la syntaxe près bien sûr...

leymiris · 10/09/2011, 16h11

Merci beaucoup ton aide est très apprécié !

Par contre je me retrouve avec un élément qui ne fonctionne pas.

dans cette partie de 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
$.each(data.collections, function(i,data){   
 
                //permet de filtrer en fonction de l'ID de la collection cliqué et si elle contient des oeuvres
                if(data.id == $id && data.total != 0){
 
 
                    $("#total-artwork").html("("+(data.total)+")");
 
                    //affichage des oeuvres de la collection choisie
                    $.each(data.arts, function(i,data){ 
 
                        //DIV des aperçus de collection et oeuvres
                        var div_artwork = div_data("artwork", data.id, data.thumbnail, data.title, data.author)
 
                        $(div_artwork).hide().appendTo(".artworks").fadeIn();
 
                    });
 
                }
 
            });

Comme tu peux le voir j'ai deux each. Le première cherche dans data.collections la collections qui est cliqué, ensuite le deuxième permet d'afficher les artworks qui sont lié à cette collection (c'est à plusieurs dimension)

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
{ "collections" :[
 
    { "id":1,
      "title":"some collection",
      "producer":"some producer",
      "director":"some director",
      "thumbnail":"image1.png",
      "year":"2011",
      "description":"some description",
      "url":"http://whatevs",
      "total": 3,
      "arts":[
      {
        "id":1,
        "sequence":1,
        "title":"figureight",
        "thumbnail":"image.png",
        "author":"Author Mc.Creator",
        "discovered":"2011-06-15 13:44:31"
      },
      {
        "id":2,
        "sequence":2,
        "title":"andy singer cartoon",
        "thumbnail":"image2.png",
        "author":"Author Mc.Creator",
        "discovered":"2011-06-15 13:44:33"
      },
      {
        "id":3,
        "sequence":3,
        "title":"post no bills",
        "thumbnail":"image3.png",
        "author":"Author Mc.Creator",
        "discovered":"2011-06-15 13:44:34"
      }]
    }
]}

c'est pour cela que je n'avais pas vraiment pensé à faire une fonction distincte :/

En tout cas merci de ton aide

galerien69 · 10/09/2011, 16h51

ah oui ok, je pige mieux.

c'est vrai que pour le coup c'est ptet mieux comme tu fais.
J'avais dans la tête le coup des listes qui s'updatent en fonction des précédentes (un peu comme des arbres). Ici, t'as quand même plus de valeur sémantique, en voyant que collections a plus d'attributs...

bref, c'est plus un non sens la factorisée en fait.

leymiris · 10/09/2011, 17h02

Peut-être est-il possible d'identifier à partir des childs (arts) à qu'elle collection ils sont associé, plutôt que de filtré les collections puis ensuite aller chercher les artworks.
Je suis certains qu'il y a une manière plus propre de faire que ce que j'ai fait

je vais continuer à chercher.

Merci encore de ton aide !

danielhagnoul · 11/09/2011, 00h20

Bonsoir

Fichier JSON utiliser pour mon test. Attention dans la page web un ID doit être unique et il doit commencer par une lettre !

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
{
	"collections" :[
		{
			"id":"collections10",
			"title":"some collection",
			"producer":"some producer",
			"director":"some director",
			"thumbnail":"image1.png",
			"year":"2011",
			"description":"some description",
			"url":"http://whatevs",
			"total": 3,
			"arts":[
				{
				  "id":"arts10",
				  "sequence":1,
				  "title":"figureight",
				  "thumbnail":"image.png",
				  "author":"Author Mc.Creator",
				  "discovered":"2011-06-15 13:44:31"
				},
				{
				  "id":"arts12",
				  "sequence":2,
				  "title":"andy singer cartoon",
				  "thumbnail":"image2.png",
				  "author":"Author Mc.Creator",
				  "discovered":"2011-06-15 13:44:33"
				},
				{
				  "id":"arts13",
				  "sequence":3,
				  "title":"post no bills",
				  "thumbnail":"image3.png",
				  "author":"Author Mc.Creator",
				  "discovered":"2011-06-15 13:44:34"
				}
			]
		},
		{
			"id":"collections20",
			"title":"some collection",
			"producer":"some producer",
			"director":"some director",
			"thumbnail":"image10.png",
			"year":"2011",
			"description":"some description",
			"url":"http://whatevs",
			"total": 3,
			"arts":[
				{
				  "id":"arts21",
				  "sequence":1,
				  "title":"figureight",
				  "thumbnail":"image.png",
				  "author":"Author Mc.Creator",
				  "discovered":"2011-06-15 13:44:31"
				},
				{
				  "id":"arts22",
				  "sequence":2,
				  "title":"andy singer cartoon",
				  "thumbnail":"image2.png",
				  "author":"Author Mc.Creator",
				  "discovered":"2011-06-15 13:44:33"
				},
				{
				  "id":"arts23",
				  "sequence":3,
				  "title":"post no bills",
				  "thumbnail":"image3.png",
				  "author":"Author Mc.Creator",
				  "discovered":"2011-06-15 13:44:34"
				}
			]
		}
	]
}

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
$.getJSON('collections.json', function(data){
	var nbCollections = data.collections.length;
 
	for (var i = 0; i < nbCollections; i++){
		var obj = data.collections[i];
 
		console.log(
			"id : ", obj.id,
			"\n title : ", obj.title,
			"\n producer : ", obj.producer,
			"\n director : ", obj.director,
			"\n thumbnail : ", obj.thumbnail,
			"\n year : ", obj.year,
			"\n description : ", obj.description,
			"\n url : ", obj.url,
			"\n total : ", obj.total
		);
 
		$.each(obj.arts, function(i, item){
			console.log(
				"id : ", item.id,
				"\n sequence : ", item.sequence,
				"\n title : ", item.title,
				"\n thumbnail : ", item.thumbnail,
				"\n author : ", item.author,
				"\n discovered : ", item.discovered
			);
		});
	}	
}).error(function(jqXHR, textStatus, errorThrown){
	// debug
	console.log(jqXHR, textStatus, errorThrown);
 
	// échec de la transaction, gèrer la catastrophe
 
});

leymiris · 12/09/2011, 04h34

Merci DanielHagnoul.
ce bout de code fonctionne bien, mais peux-tu m'expliquer en qui il serait préférable d'utiliser ce que tu as mentionné ? La différence que je vois par rapport à ce que j'ai fait c'est que pour le 1er each tu utilises une boucle for.

danielhagnoul · 12/09/2011, 11h37

Bonjour

Il y a plusieurs différences avec votre version.

Depuis qu'une transaction AJAX retourne un objet différé (Deferred) on dispose d'une écriture simplifiée pour traiter l'erreur.

Code :

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

1
2
3
4
5
6
7
8
9
$.getJSON('collections.json', function(data){
	// succès traitement de data
 
}).error(function(jqXHR, textStatus, errorThrown){
	// debug
	console.log(jqXHR, textStatus, errorThrown);
 
	// échec de la transaction, gèrer la catastrophe
});

Voir : http://javascript.developpez.com/faq...ax#AjaxRequest
et http://javascript.developpez.com/faq...#ObjetDeferred

J'ai utilisé un fichier JSON contenant deux collections pour m'assurer de la validité du code. J'ai modifié les ID qui étaient non conformes.

Une fois le JSON parsé par $.getJSON() qui utilise en interne $.parseJSON() (http://api.jquery.com/jQuery.parseJSON/) on obtient dans la variable data un objet JavaScript utilisant la notation JSON (http://www.json.org/js.html).

Cet objet contient un tableau (array collections) d'objets anonymes ({...}). Il m'a donc semblé naturel de prendre la longueur du tableau (array) et de le parcourir avec une boucle for() directement plutôt que de passer par une instruction $.each() qui en interne utilise une boucle for(). On ne doit jamais oublier que jQuery c'est du JavaScript.

Maintenant, vous pouvez me dire que obj.arts est également un tableau (array) contenant des objets anonymes et que là pourtant j'utilise un $.each() alors que j'aurais pu utiliser directement une boucle for(). Oui ! Mais je suis comme tout le monde, un peu fainéant, et l'écriture avec $.each() est plus simple.

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
$.getJSON('collections.json', function(data){
	var nbCollections = data.collections.length,
		i,
		j;
 
	for (i = 0; i < nbCollections; i++){
		var obj = data.collections[i];
 
		console.log(
			"id : ", obj.id,
			"\n title : ", obj.title,
			"\n producer : ", obj.producer,
			"\n director : ", obj.director,
			"\n thumbnail : ", obj.thumbnail,
			"\n year : ", obj.year,
			"\n description : ", obj.description,
			"\n url : ", obj.url,
			"\n total : ", obj.total
		);
 
		var long = obj.arts.length;
 
		for (j = 0; j < long; j++){
			console.log(
				"id : ", obj.arts[j].id,
				"\n sequence : ", obj.arts[j].sequence,
				"\n title : ", obj.arts[j].title,
				"\n thumbnail : ", obj.arts[j].thumbnail,
				"\n author : ", obj.arts[j].author,
				"\n discovered : ", obj.arts[j].discovered
			);
		}
	}	
}).error(function(jqXHR, textStatus, errorThrown){
	// debug
	console.log(jqXHR, textStatus, errorThrown);
 
	// échec de la transaction, gèrer la catastrophe
 
});

Dans ce cas, j'ai utilisé les variables i et j car on ne doit pas écraser i dans le second for().

okoweb · 12/09/2011, 18h58

Citation:

Envoyé par danielhagnoul

Attention dans la page web un ID doit être unique

Oui

Citation:

Envoyé par danielhagnoul

il doit commencer par une lettre !

Je ne comprends pas cela et surtout dans ce cas ?

Code :

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

1
2
 
data.collections[0].id // retourne bien 1

Où est le mal ?

danielhagnoul · 12/09/2011, 19h12

Citation:

Envoyé par okoweb

Je ne comprends pas cela et surtout dans ce cas ?

Code :

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

1
2
 
data.collections[0].id // retourne bien 1

Où est le mal ?

C'est la norme HTML 4 qui demande qu'un ID commence par une lettre.

Pardon ?

Vous n'avez pas testé et regardé ce que retournent les console.log() !

id : collections10 
title : some collection 
producer : some producer 
director : some director 
thumbnail : image1.png 
year : 2011 
description : some description 
url : http://whatevs
total : 3

id : arts10 
sequence : 1 
title : figureight 
thumbnail : image.png 
author : Author Mc.Creator 
discovered : 2011-06-15 13:44:31

...

okoweb · 13/09/2011, 10h08

Citation:

Envoyé par danielhagnoul

C'est la norme HTML 4 qui demande qu'un ID commence par une lettre.

Dans ce cas oui.
Mais la plupart des temps ce genre de collections JSON se génère depuis un langage serveur qui puise dans bdd. Et dans ce cas les id sont toujours des nombres entiers. L'essentiel c'est que ces identifiants soient uniques.
Et je ne vois vraiment pas de différence entre :

id : arts10 
id : 10

sauf explicitement arts pourrait signifier artiste, mais dans la manipulation de notre JSON RAS.

Meilleures salutations.

10/09/2011, 16h51	#4
galerien69 Membre éclairé F5(){F5} Inscription : avril 2008 Messages : 256 Détails du profil Informations personnelles : Sexe : Localisation : France, Rhône (Rhône Alpes) Informations professionnelles : Activité : F5(){F5} Secteur : High Tech - Éditeur de logiciels Informations forums : Inscription : avril 2008 Messages : 256 Points : 320 Points : 320	ah oui ok, je pige mieux. c'est vrai que pour le coup c'est ptet mieux comme tu fais. J'avais dans la tête le coup des listes qui s'updatent en fonction des précédentes (un peu comme des arbres). Ici, t'as quand même plus de valeur sémantique, en voyant que collections a plus d'attributs... bref, c'est plus un non sens la factorisée en fait.
	00

10/09/2011, 17h02	#5
leymiris Invité de passage Inscription : novembre 2008 Messages : 32 Détails du profil Informations forums : Inscription : novembre 2008 Messages : 32 Points : 2 Points : 2	Peut-être est-il possible d'identifier à partir des childs (arts) à qu'elle collection ils sont associé, plutôt que de filtré les collections puis ensuite aller chercher les artworks. Je suis certains qu'il y a une manière plus propre de faire que ce que j'ai fait je vais continuer à chercher. Merci encore de ton aide !
	00

12/09/2011, 04h34	#7
leymiris Invité de passage Inscription : novembre 2008 Messages : 32 Détails du profil Informations forums : Inscription : novembre 2008 Messages : 32 Points : 2 Points : 2	Merci DanielHagnoul. ce bout de code fonctionne bien, mais peux-tu m'expliquer en qui il serait préférable d'utiliser ce que tu as mentionné ? La différence que je vois par rapport à ce que j'ai fait c'est que pour le 1er each tu utilises une boucle for.
	00

Outils de la discussion
Afficher une version imprimable Envoyer un lien vers cette page par email