Discussion :

[snake264]

J'ai un problème lors du tri de ma map par valeur, je tombe sur une exception NullPointerException et je n'arrive pas à voir où ce trouve mon erreur.

Voici le code que j'utilise :

Code :

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

1
2
3
4
5
6
7
8
9
public void display () {
	List<String> keys = new ArrayList<> (this.list.keySet ());
 
	Collections.sort (keys, new ValueComparator (this.list));
 
	for (String str : keys){
	    System.out.println ("string : " + str + " count : " + this.list.get (str));
	}
}

Ma classe implémentant l'interface Comparator :

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
public class ValueComparator implements Comparator<String> {
    private Map<String, Integer> map;
 
    public ValueComparator (Map<String, Integer> map) {
	this.map.putAll (map);
    }
 
    @Override
    public int compare (String sourceKey, String targetKey) {
	int sourceValue = Integer.valueOf (this.map.get (sourceKey));
	int targetValue = Integer.valueOf (this.map.get (targetKey));
 
	if (sourceValue > targetValue) {
	    return 1;
	}
	else if (sourceValue > targetValue) {
	    return -1;
	}
 
	return 0;
    }
}

Une idée de ce que j'aurai fait de traver ?

par avance

[thelvin]

Le plus simple est de regarder la stacktrace de l'exception : elle te dit à quel endroit et à quelle ligne l'exception est survenue.

Donc, avec ton code, cela arrive probablement ici :

Code :

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

1
2
3
public ValueComparator (Map<String, Integer> map) {
  this.map.putAll (map);
}

Tu n'as pas initialisé this.map, qui donc est null, et appeler une méthode dessus (que ce soit putAll() ou quoi que ce soit) génèrera un NullPointerException.


À part ça :
- je vois au moins un bug évident dans le comparateur,
- je ne vois pas l'intérêt de Integer.valueOf().

[snake264]

exact c'était bien ça. Le bug en effet je faisais deux fois la même comparaison, et pour la seconde remarque parce que c'est plus naturel pour moi de manipuler des int

bien pour ton aide

[tchize_]

Je rajouterais ceci:

1) pourquoi créer une nouvelle map alors que la map existante suffit?
2) pourquoi tout ce bordel dans compare alors qu'il est identique à

Code :

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

1
2
3
    public int compare (String sourceKey, String targetKey) {
        return map.get (sourceKey).compareTo(map.get (targetKey));
    }

et que donc au final ton code peux se résumer à

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
13
14
public void display () {
	List<String> keys = new ArrayList<> (this.list.keySet ());
 
	Collections.sort (keys, new Comparator<String> (this.list){
               @Override
               public int compare (String o1, String o2) {
                     return list.get(o1).compareTo(list.get(o2));
               }
	});
 
	for (String str : keys){
	    System.out.println ("string : " + str + " count : " + this.list.get (str));
	}
}

3) c'est une très mauvaise idée d'appeler ta map "list", écrire

Code :

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

Map list = ....

C'est une aussi mauvaise idée que d'écrire

Code :

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

double entier = ....

Ca rend le code très dur à lire

[snake264]

1) pourquoi créer une nouvelle map alors que la map existante suffit?

C'est-à-dire ?

2) pourquoi tout ce bordel dans compare alors qu'il est identique à

Code :

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

1
2
3
    public int compare (String sourceKey, String targetKey) {
        return map.get (sourceKey).compareTo(map.get (targetKey));
    }

Exact, je suppose que la méthode "compareTo" vient de l'interface "Comparable" c'est ça ?

et que donc au final ton code peux se résumer à

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
13
14
public void display () {
	List<String> keys = new ArrayList<> (this.list.keySet ());
 
	Collections.sort (keys, new Comparator<String> (this.list){
               @Override
               public int compare (String o1, String o2) {
                     return list.get(o1).compareTo(list.get(o2));
               }
	});
 
	for (String str : keys){
	    System.out.println ("string : " + str + " count : " + this.list.get (str));
	}
}

Non car mon but est de me faire un comparateur générique, qui prenant une map contenant n'importe quel type de valeurs, là j'ai pris String juste pour avoir quelque chose de précis à tester.

3) c'est une très mauvaise idée d'appeler ta map "list", écrire

Code :

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

Map list = ....

C'est une aussi mauvaise idée que d'écrire

Code :

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

double entier = ....

Ca rend le code très dur à lire

Je sais mais quand je développe un petit truc exclusivement réservé à mon usage personnel et que personne d'autres n'utilisera je ne fais pas attention à ce genre de chose

Si je prévois de le publier ou de le donner à quelqu'un d'autre oui je nommerai mieux et je commenterai (car je commente que très rarement mes codes perso aussi). Mais quand je code pour moi je code très salement d'une manière souvent que ya que moi qui comprenne

[tchize_]

C'est-à-dire ?

Que tu fais une copie de la map qu'on te passe (création d'une nouvelle map puis appel de addAll) alors que tu ne la modifie pas -> conserve celle qu'on te passe tout simplement.

Exact, je suppose que la méthode "compareTo" vient de l'interface "Comparable" c'est ça ?

oui

[snake264]

C'est vrai, si j'utilise compareTo je n'ai plus besoin de copier la map

en tout cas pour tes conseils et remarques