Discussion :

[ludovic01]

Bonsoir,

Je cherche à améliorer le code de mon application afin de le rendre propre et clair. L'objectif final étant de coder un jeu de memory.

Ci-dessous une partie de code commenté :

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
// nombreImageTotal, est une variable qui dépend d'une autre classe, qui varie entre min16 et de environs 20 (cela dépend de moi et est codé en dur.
    private static JLabel[][] tabTotal = new JLabel[2][nombreImageTotal * 2]; //le *2 sert parce que je souhaite avoir deux fois la même image.
    public static ArrayList<JLabel> list = new ArrayList<>();
    public static ArrayList<JLabel> listDos = new ArrayList<>();
 
//ici la fonction qui servira à créer des labels. Je la poste en premier pour plus de clarté.
public static JLabel createJLabel (String i){
    JLabel jl = new JLabel(new ImageIcon(CreationVariable.class.getResource("/image/" + getTheme() + i + ".png")));
    return jl;
}
 
//Création de la première partie du tableau, car je n'ai pas trouvé le moyen de le faire en une seule fois...
// aussi un point important, je pose un "setText" qui est vital pour la suite.
for(int i = 0 ; i < 2 ; i++){
        for ( int j = 0 ; j <  nombreImageTotal; j++ ){
            if ( i == 0 ) {
            tabTotal[i][j] = createJLabel( Integer.toString(j+1) );
            tabTotal[i][j].setText(j+1); 
            }else{
            tabTotal[i][j] = createJLabel( "dos" );
            tabTotal[i][j].setText((j+1);
        }}
    }
 
//dans cette deuxième partie je n'ai pas trouvé mieux que de refaire une boucle afin de simplement "doubler" le tableau
 
    for(int i = 0 ; i < 2 ; i++){
        for ( int j = nombreImageTotal ; j < nombreImageTotal * 2 ; j++ ){
            if ( i == 0 ) {
            tabTotal[i][j] = createJLabel( Integer.toString(j+1-nombreImageTotal )); 
            tabTotal[i][j].setText(j+1- nombreImageTotal);
            }else{
            tabTotal[i][j] = createJLabel( "dos" );
            tabTotal[i][j].setText(j+1- nombreImageTotal);
        }}
    }
 
// Ces deux listes sont clear() dans une autre partie du programme
// elles me servent par la suite à insérer ces jlabel dans un panel dans une boucle.
    for(int i = 0; i < nombreImageTotal * 2 ; i++){
        list.add(tabTotal[0][i]);       // [0][i] sert a récupérer la liste d'image.
        listDos.add(tabTotal[1][i]);} // [1][i] sert pour récupérer une image de carte retourné avec le fameux 'setText" identique à la ligne de dessus
    //NiveauDifficultee est fixé par un radio bouton dans un menubar et en cas de modification en cour de partie, relance "l'initialisation" du jeux.
    int b = (list.size() / 2) - (NiveauDifficultee / 2);
    int d = nombreImageTotal;
//ici je tire des cartes aléatoirement que j'efface de mes listes, mais comme j'ai deux fois les mêmes cartes je dois effacer deux fois.
    for(int i = 0 ; i < b ; i++) {
        int a = (int) (Math.random() * (list.size() / 2) );
        list.remove(a + d);
        list.remove(a);
        listDos.remove(a + d);
        listDos.remove(a);
        d--; }
 
// j'ai pompé ce code sur le net, mais je n'ai pas compris comment il fonctionnait
// ce que je sais, c'est qu'il permet de mélanger plusieurs ArrayList en même temps sans permutation
    long seed = System.nanoTime();
    Collections.shuffle(list, new Random(seed));
    Collections.shuffle(listDos, new Random(seed));

Pour ceux que ça intéresse, voici la boucle qui affiche les deux ArrayList :

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
    for(int i = 0; i < list.size() ; i++){
 
       listDos.get(i).setBounds(tabLabX[i], tabLabY[i], 150, 200);
       listDos.get(i).setName("" + (i));
       list.get(i).setBounds(tabLabX[i], tabLabY[i], 150, 200);
       list.get(i).setName("" + (i));
       list.get(i).setVisible(false);
    listDos.get(i).addMouseListener(new ListenerPerso());
 
    getPanJeux().add(listDos.get(i));
    getPanJeux().add(list.get(i));
    }

Ainsi que le Listener.

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
static Boolean clique = false;
    static public String textClique;
    static public String nomClique;
    static public String ancienTextClique;
    static public String ancienNomClique;
    static public int NombreCarteTrouver;
 
    int NiveauDifficultee = CreationVariable.getNiveauDifficultee();
    ArrayList<JLabel> list = CreationVariable.getList();
    ArrayList<JLabel> listDos = CreationVariable.getListDos();
 
    public void mouseClicked(MouseEvent me) {
 
 
        textClique = ((JLabel) me.getSource()).getText();
        nomClique = ((JLabel) me.getSource()).getName();
//        System.out.println(textClique);
//        System.out.println(nomClique);
 
        listDos.get(Integer.parseInt(nomClique)).setVisible(false);
        list.get(Integer.parseInt(nomClique)).setVisible(true);
 
        if ( clique == true) {
            try {
                Thread.sleep(100);
            } catch (InterruptedException e1) {            //cette tempo est juste un test car il y a des fois certains bugs.
                // TODO Auto-generated catch block
                e1.printStackTrace();
            }
            clique = false;
            if ( textClique.equals(ancienTextClique) == false ) {
                String az = ancienNomClique;
                String aze = nomClique;
                new TimerEDT (900, e-> (list.get(Integer.parseInt(aze)).setVisible(false)));
                new TimerEDT (900, e-> (list.get(Integer.parseInt(az)).setVisible(false)));
                new TimerEDT (900, e-> (listDos.get(Integer.parseInt(aze)).setVisible(true)));
                new TimerEDT (900, e-> (listDos.get(Integer.parseInt(az)).setVisible(true)));
            }else {
                NombreCarteTrouver = NombreCarteTrouver +2;
 
                if (NombreCarteTrouver == NiveauDifficultee) {
                    System.out.println("Gagner");
                    Fenetre.getPanJeux().removeAll();
 
                    JLabel vic = new JLabel(new ImageIcon(CreationVariable.class.getResource("/image/victoire.png")));
                    vic.setBounds(10, 10, 429, 415);
                    vic.setVisible(true);
                    Fenetre.getPanJeux().add(vic);
                }
 
            }
 
        }else {
            clique = true;
 
        }
        ancienTextClique = textClique;
        ancienNomClique = nomClique;
 
//        Thread t = new ThreadTest();
//        t.start();
 
          Fenetre.getPanJeux().repaint();
   }

Il est important de noter que j'en suis seulement à la version "preview" surtout dans le code du listener et que la question principale se trouve dans le premier "code".

Quelqu'un saurait-il m'indiquer comment améliorer celui-ci ?

Merci d'avance pour votre aide.

[joel.drigo]

Salut,

Déjà, au lieu de traiter texte et image de JLabel séparement, pourquoi ne pas avoir une méthode qui gère les 2 :

Code :

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

1
2
3
4
5
public static JLabel createJLabel (String text, String iconId){
    JLabel label = new JLabel(new ImageIcon(CreationVariable.class.getResource("/image/" + getTheme() + iconId + ".png"))); 
    label.setText(text);
    return label;
}

Ensuite,

1. Tu boucles j de 0 à n exclus, mais l'indice de boucle est toujours utilisé sous forme j+1 : on peut boucler de 1 à n inclus du coup
2. Ensuite, la première ligne et la seconde sont manifestement différentes : au lieu de faire un test à chaque itération, on peut traiter les 2 séparément
3. Ensuite, la première partie (<nombreTotalImage) et la second (>=nombreTotalImages) sont similaires sauf pour les indices, au lieu de traiter les paramètres à passer à createLabel, on peut traiter aussi l'indice dans le tableau

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
13
14
 
// première ligne
for ( int j = 1 ; j <=  nombreImageTotal; j++ ) { // comme on utilise toujours j+1, autant boucle de 1 à n inclus
	String text = String.valueOf(j);
	tabTotal[0][j] = createJLabel( text, text ); // correspond à la première partie
    	tabTotal[0][j+nombreImageTotal] = createJLabel( text, text);  // correspond à la seconde partie
}
 
// seconde ligne
for ( int j = 1 ; j <=  nombreImageTotal; j++ ){
	String text = String.valueOf(j);
    	tabTotal[1][j] = createJLabel( text, "dos" );
    	tabTotal[1][j+nombreImageTotal] = createJLabel( text, "dos" );
}

Maintenant que les boucles sont simplifiées, on peut voir qu'on pourrait les fusionner :

Code :

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

1
2
3
4
5
6
7
for ( int j = 1 ; j <=  nombreImageTotal; j++ ) { 
	String text = String.valueOf(j);
	tabTotal[0][j] = createJLabel( text, text ); 
    	tabTotal[0][j+nombreImageTotal] = createJLabel( text, text);  
    	tabTotal[1][j] = createJLabel( text, "dos" ); 
    	tabTotal[1][j+nombreImageTotal] = createJLabel( text, "dos" );
}

Pour transférer les tableaux dans les listes tu pourrais faire :

Code :

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

1
2
list.addAll( Arrays.asList(tabTotal[0]));
listDos.addAll( Arrays.asList(tabTotal[1]));

Mais comme les deux tableaux/listes doivent être synchro (ou solidaires, on va dire), pourquoi ne pas faire une liste de tableaux ? Ce qui ferait que la synchro entre les 2 listes serait implicite, puisqu'il n'y en aurait qu'une liste. Le problème, c'est que si on écrit :

Code :

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

List<JLabel[]> list = new ArrayList<>(Arrays.asList(tabTotal));

on ne va pas avoir une liste de tableaux de 2 cases (des colonnes), mais une liste de 2 tableaux de nombeImageTotal*2 cases (des lignes). Pour résoudre ce problème, il suffit lorsqu'on créé le tableau d'inverser lignes et colonnes (au lieu de [0][j], on fait [j][0] - il faut inverser évidemment lors du dimensionnement).

NB : je fais un new ArrayList<>() alors que je pourrais directement faire : List<JLabel[]> list = new ArrayList<>(Arrays.asList(tabTotal));. Pourquoi ? Parce que la liste qui représente le tableau transformée en liste est liée au tableau : donc on ne peut supprimer de valeurs dedans (un tableau ne peut changer de taille).

[ludovic01]

Bonsoir,

Encore une fois un très gros merci pour cette réponse limpide.

Voici le code final

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
	private static JLabel[][] tabTotal = new JLabel[nombreImageTotal * 2][2];
	private static List<JLabel[]> list = new ArrayList<>(Arrays.asList());
 
public static void inisialiseListe () {
 
	for ( int j = 0 ; j <  nombreImageTotal; j++ ){
		tabTotal[j][0] = createJLabel( Integer.toString(j) , j );
		tabTotal[j+nombreImageTotal][0] = createJLabel( Integer.toString(j) , j);
		tabTotal[j][1] = createJLabel( "dos" , j );
		tabTotal[j+nombreImageTotal][1] = createJLabel( "dos" , j ); }
 
	setList(new ArrayList<>(Arrays.asList(tabTotal)));
 
	int d = nombreImageTotal;
	do {
		int a = (int) (Math.random() * (list.size() / 2) );
		list.remove(a+d);
		list.remove(a);
		d--; 
	}while (list.size() != niveauDifficultee );
 
Collections.shuffle(list);
}
 
public static JLabel createJLabel (String i , int j){
	JLabel jl = new JLabel(new ImageIcon(CreationVariable.class.getResource("/image/" + getTheme() + i + ".png")));
	jl.setText(Integer.toString(j));
    return jl;
 
 
public static List<JLabel[]> getList() {
	return list;
}
 
public static void setList(List<JLabel[]> List) {
	list = List;
}
}

Par contre ce n'était pas simple de trouver les bonnes syntaxes...

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
	for(int i = 0; i < CreationVariable.getList().size() ; i++){
 
		CreationVariable.getList().get(i)[1].setBounds(tabLabX[i], tabLabY[i], 150, 200);
		CreationVariable.getList().get(i)[1].setName("" + (i));
		CreationVariable.getList().get(i)[1].addMouseListener(new ListenerPerso());
		CreationVariable.getList().get(i)[0].setBounds(tabLabX[i], tabLabY[i], 150, 200);
		CreationVariable.getList().get(i)[0].setName("" + (i));
		CreationVariable.getList().get(i)[0].setVisible(false);
 
		getPanJeux().add(CreationVariable.getList().get(i)[1]);
		getPanJeux().add(CreationVariable.getList().get(i)[0]);
    }

Encore un très gros merci;

[joel.drigo]

Au lieu d'un for, tu peux utiliser un foreach, qui t'évite déjà d'avoir un get(i) :

Code :

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

1
2
3
4
5
6
7
8
9
10
11
 
int i=0;
for(JLabel[] labels : CreationVariable.getList()) {
 
    labels[1].setBounds(tabLabX[i], tabLabY[i], 150, 200);
 
/*.... */
 
 
    i++;
}

Ensuite, pour éviter des notations de tableaux, on peut utiliser une classe qui a 2 attributs avec les 2 JLabels, ou tout simplement une méthode init(). Tu mets tout le code d'initialisation dans une méthode init(), en faisant des méthodes pour séparer les tâches (plus les méthodes font moins de choses, plus c'est simple, si je puis dire, et plus c'est facile de factoriser aussi )

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
 
   private void init(JLabel label1, JLabel label2, String name, int , int y) {
        label2.setBounds(x, y, 150, 200);
	label2.setName(name);
	label2.addMouseListener(new ListenerPerso());
 
        label1..setBounds(x, y, 150, 200);
	label1.setName(name);
	label1.setVisible(false);
   }
 
}

On voit immédiatement comment factoriser :

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
13
private void init(JLabel label1, JLabel label2, String name, int , int y) {
        setLabel(label1, name, x, y);
        setLabel(label2, name, x, y);
	label1.setVisible(false);
	label2.addMouseListener(new ListenerPerso());
   }
 
   private void setCommon(JLabel label, int x, int y) {
        label.setBounds(x, y, 150, 200);
	label.setName(name);
   }
 
}

Bon, dans ton cas, ce n'est pas forcément nécessaire parce qu'il y a que 2 lignes en commun, mais si tu avais 4 ou 5 appels d'initialisation en commun, ça commencerait à devenir utile pour simplifier le code, et y voir plus clair.

[ludovic01]

ok, je vois. Je pense que je commence à saisir le principe.
Par contre je pense laisser mon code tel qu'il est, par contre je garde cette solution de coté.

Merci encore.