Discussion :

[ceres02]

Bonjour,

je cherche a trier alphabetiquement une liste de fichier en utilisant bien evidemment leurs noms.

J'utilise la classe Comparator avec un code specifique pour qu'au final la liste soit trier de telle facon que les dossiers soient reorganises alphabetiquement mais situe en premier dans la liste puis ensuite les fichiers.

Les dossiers sont biens tries mais quand on en vient au fichier, il y a un groupe de 4/5 fichiers dans la liste qui se retrouve trier mais en dernier dans la liste.

J'ai essaye un simple compararTo uniquement sur la liste des fichiers et j'obtiens le meme reordering donc ce n'est apparemment ma methode qui distingue fichier ou dossier qui pose probleme lorsque je les trie.

En revanche, si je fais un Collections.sort(Liste), la les fichiers sont bien reorganises alphabetiquement.....

Donc le probleme vient bien du comparator mais c est bizarre car les nombreux exemple que j ai pu voir pour comparer des strings implementait uniquement un return string1.compareTo(string2)

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
 public class NameComparator implements Comparator, Serializable{
            public int compare(Object fileName1, Object fileName2){
 
                int response=0;
                File file1 = (File)fileName1;
                File file2 = (File)fileName2;
 
                String name1 = file1.getName();
                String name2 = file2.getName();
 
                if(name1.equals("Parent Directory")) response = 1;
                else if(name2.equals("Parent Directory")) response = -1;
 
                else if(file1.isDirectory() && file2.isFile()) response = -1;
                else if(file2.isDirectory() && file1.isFile()) response = 1;
 
                else if(file1.isDirectory() && file2.isDirectory()) {
                    if(name1.compareTo(name2) > 0) response = 1;
 
                }
                else if(file1.isFile() && file2.isFile()) {
                    if(name1.compareTo(name2) > 0) response = 1;   
                }
                if(name1.compareTo(name2) > 0) response = 1;
                else response =-1;
               //System.out.println(name1 + " comparedTo " + name2 + " : " + String.valueOf(response));
            return response;
 
            }

Merci pour votre aide et bonne journee,

Antoine.

[vahid]

Salut,

Tout d'abord on code n'est pas très lisible :s, cet enchainement de else if sans indentation ne facilite pas la comprehension :/, y a t il une raison au fait que tu ne casse pas ta fonction avec plusieurs returns à des endroits strategiques ?

Quand à ton probleme (à ce que j'ai pu comprendre a ton code apres l avoir indenté), il semblerait que la derniere comparaison

Code :

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

1
2
3
4
5
6
7
 
			if(name1.compareTo(name2) > 0) 
				response = 1;
			else 
				response =-1;
			//System.out.println(name1 + " comparedTo " + name2 + " : " + String.valueOf(response));
			return response;

prenne totalement la main sur toutes les comparaisons effectuées auparavant : Peu importe ce que tu as fait avant, la fonction va passer dans cette boucle apres l enchainement de else if et ecraser les resultats precedants.

Je te conseille fortement de return un resultat des que tu es sur d'en avoir un au lieu de poursuivre la fonction dans un else.

a+

[aJavaDeveloper]

Tout d'abord ton code n'est pas très lisible :s, cet enchainement de else if sans indentation ne facilite pas la comprehension :/

Là je dois dire que je ne suis pas du tout d'accord avec toi : sa mise en forme des else if permet de rendre son code plus compact et plus lisible.

y a t il une raison au fait que tu ne casse pas ta fonction avec plusieurs returns à des endroits strategiques ?

Pourquoi casserait-il sa fonction avec des return puisqu'il fait

Code :

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

1
2
3
4
5
if ... 
else if ... 
else if ... 
else if ...
...

Il n'y aura jamais qu'un seul cas qui sera vérifié (grâce aux else).

il semblerait que la derniere comparaison prenne totalement la main sur toutes les comparaisons effectuées auparavant

C'est tout à fait correcte : je pense que ceres02 a oublié le else avant

Code :

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

if(name1.compareTo(name2) > 0)

[vahid]

Là je dois dire que je ne suis pas du tout d'accord avec toi : sa mise en forme des else if permet de rendre son code plus compact et plus lisible.

Pourquoi casserait-il sa fonction avec des return puisqu'il fait

Question d'habitude, personnellement je pense qu'a la place d'un else/if/affectation, un if/return est plus adapté.

Question d'habitude, comme je dis, et ca aurait éviter l'écrasement de la dernière comparaison

enfin bon on va pas se battre pr ca

[aJavaDeveloper]

Question d'habitude, personnellement je pense qu'a la place d'un else/if/affectation, un if/return est plus adapté.

C'est vrai que, pour ma part, j'ai également plus tendance à utiliser le if/return.
Toutefois, j'ai déjà entendu dire (peut être à tort) qu'une procédure avec plusieurs points de sortie (= plusieurs return) est un anti-pattern.

Question d'habitude, comme je dis, et ca aurait éviter l'écrasement de la dernière comparaison

C'est vrai...

enfin bon on va pas se battre pr ca

Evidemment que non .
Mais une discussion impliquant des points de vue contraires est toujours enrichissantes.

[ceres02]

Bonjour,

je suis desole pour la reponse tardive mais comme je ne voyais pas de reponses les premiers jours, j'ai cru que c etait peine perdue =)

Pour ce qui est de l ecriture du code... j'ai tendance a fonctionner comme aJavaDeveloper, c est a dire faire un code compact.

Si j ai plusieurs lignes a rentrer dans un if, les accolades sont les biens venus mais si j en ai qu'une, je sais que pour cette condition, je trouve a cote l action et en plus j ai pas 15.000 lignes de codes avec des {}.

Maintenant pour ce qui est de mon probleme, j avais perdu tout espoir et vos reponses m'ont relance. J'ai effectivement oublie un else la ou vous l avez mentionne. Mais ce n est pas la ou le probleme se situait.

Il faut utilise des equals compareToIgnoreCase() et non pas des compare(), tout simplement. Ca marche parfaitement et je vous remercie quand meme pour les suggestions car sans vous je m y serai pas replonge =).

Ainsi, voici le code en question pour ceux que ca interesse, grandement simplifie et beaucoup plus clair:

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
13
public class NameComparator implements Comparator, Serializable{  
         public int compare(Object f1, Object f2){  
             FileSystemViewer fsv = FileSystemViewer.getFileSystemView();
             File file1 = (File)f1;
             File file2 = (File)f2;
             boolean dirF1 = file1.isDirectory();
             boolean dirF2 = file2.isDirectory();
 
             if(dirF1 && !dirF2) return -1; 
             else if(dirF2 && !dirF1) return 1; 
             else return fsv.getSystemDisplayName(file1).compareToIgnoreCase(fsv.getSystemDisplayName(file2));            
         }
     }

[miloux32]

Je rebondis sur les remarques .

Perso ( et ce n'est que mon avis) on m'a toujours appris à limiter au maximum au maximum les points de rertours.

Un return par fonction/méthode c'est la régle.
Sinon ca veut dire que votre code est mal organisé et que vous êtes obligé de tout modifier pour modifier la sortie.

( Genre vous retournez un int , et ca devient un float, il faut tout recaster. )