NullPointerException lors du tri d'une map

Version imprimable

:salut:

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:

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:

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 ?

:merci: par avance :)

01/08/2012, 21h51
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:

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().
01/08/2012, 22h30
snake264

:oops: 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:aie:

:merci: bien pour ton aide :)

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:

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:

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:

Map list = ....

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

Code:

double entier = ....

Ca rend le code très dur à lire :mrgreen:

Citation:

Envoyé par tchize_

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

C'est-à-dire ?
Citation:
Envoyé par tchize_
2) pourquoi tout ce bordel dans compare alors qu'il est identique à
Code:

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 ?
Citation:
Envoyé par tchize_
et que donc au final ton code peux se résumer à
Code:

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.

Citation:

Envoyé par tchize_

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

Code:

Map list = ....

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

Code:

double entier = ....

Ca rend le code très dur à lire :mrgreen:

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:mrgreen:

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:aie:

02/08/2012, 00h03
tchize_

Citation:

Envoyé par snake264

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.

Citation:

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

oui
02/08/2012, 00h54
snake264

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

:merci: en tout cas pour tes conseils et remarques:ccool: