Discussion :

[adrien881]

Bonjour à tous,

Je ne comprends pas un bogue trouvé par findBug.

Voici mon code :

Code :

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

1
2
3
4
5
6
7
8
 
public class Test implements Comparable<Object> {
	int attribut;
 
	public int compareTo(Object o) {
		return attribut - ((Test) o).attribut;
	}
}

findBug dit «Test defines compareTo(Object) and uses Object.equals()».

Voici la description du bogue sur le site de findBug*:
http://findbugs.sourceforge.net/bugD..._OBJECT_EQUALS

Pour moi la condition

Code :

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

(x.compareTo(y)==0) == (x.equals(y))

est bien respectée.

Est-ce une erreur de findBug? Car il retourne le même message sur le code du livre Pensez en Java*:
http://bruce-eckel.developpez.com/li...chap=10&page=2

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
//: c09:CompType.java
// Implémenter Comparable dans une classe.
import com.bruceeckel.util.*;
import java.util.*;
 
public class CompType implements Comparable {
  int i;
  int j;
  public CompType(int n1, int n2) {
    i = n1;
    j = n2;
  }
  public String toString() {
    return "[i = " + i + ", j = " + j + "]";
  }
  public int compareTo(Object rv) {
    int rvi = ((CompType)rv).i;
    return (i < rvi ? -1 : (i == rvi ? 0 : 1));
  }
  private static Random r = new Random();
  private static int randInt() {
    return Math.abs(r.nextInt()) % 100;
  }
  public static Generator generator() {
    return new Generator() {
      public Object next() {
        return new CompType(randInt(),randInt());
      }
    };
  }
  public static void main(String[] args) {
    CompType[] a = new CompType[10];
    Arrays2.fill(a, generator());
    Arrays2.print("before sorting, a = ", a);
    Arrays.sort(a);
    Arrays2.print("after sorting, a = ", a);
  }
} ///:~

Merci pour vos réponses…

[Mamypirate]

Je pense que tu devrais faire :

Code :

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

1
2
3
4
5
6
7
public class Test implements Comparable<Test> {
	int attribut;
 
	public int compareTo(Test t) {
		return attribut - t.attribut;
	}
}

[adrien881]

Merci pour votre réponse Mamypirate mais hélas avec votre code j’ai toujours le même message.

[-gma-]

Bonjour,

Pour moi la condition

Code :

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

(x.compareTo(y)==0) == (x.equals(y))

est bien respectée.

Pas pour moi

Code :

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

1
2
3
4
5
6
7
8
9
Test x = new Test();
Test y = new Test();
x.attribut = 5;
y.attribut = 5;
 
System.out.println(x.compareTo(y)); // 0
System.out.println(x.equals(y)); // false
// Donc
System.out.println((x.compareTo(y)==0)  + " == " + (x.equals(y))); // true == false

En fait il te suggère de surcharger la méthode equals() par ce que celle qui est héritée de Object ne return true que s'il s'agit de la même instance.

[adiGuba]

Salut,

Pour moi la condition

Code :

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

(x.compareTo(y)==0) == (x.equals(y))

est bien respectée.

Non puisque tu ne redéfinis pas la méthode equals(). C'est justement cela qui t'es signalé par FindBugs.

compareTo() va se baser sur la valeur de l'attribut, tandis que equals() va se baser sur l'identité de l'objet (implémentation par défaut hérité de la classe Object).

Donc il faut redéfinir la méthode equals() ! Et au passage tu devrais plutôt implémenter Comparable<Test> :

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
public class Test implements Comparable<Test> {
	int attribut;
 
	public int compareTo(Test other) {
		return this.attribut - other.attribut;
	}
 
	@Override
	public boolean equals(Object obj) {
		if (obj==this)
			return true;
		if (obj instanceof Test) {
			Test other = (Test) obj;
			return this.attribut == other.attribut;
		}
		return false;
	}
}

A noter enfin qu'il serait souhaite d'implémenter également la méthode hashCode()...


a++

[edit] grillé, mais de peu

[Vakhyw]

D'accord avec adiGuba. Néanmoins je changerais ça :

Code :

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

1
2
3
4
5
 
if (obj instanceof Test) {
	Test other = (Test) obj;
	return this.attribut == other.attribut;
}

en :

Code :

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

1
2
3
4
 
if (obj instanceof Test) {
        return (compareTo((Test) obj) == 0);
}

Ca te permet de factoriser du code ;-)

[adrien881]

Merci,

Mon code devient alors:

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
public class Test implements Comparable<Test> {
	int attribut;
 
	public int compareTo(Test o) {
		return attribut - o.attribut;
	}
 
	@Override
	public boolean equals(Object obj) {
		if (obj == this)
			return true;
		if (obj instanceof Test) {
			return (compareTo((Test) obj) == 0);
		}
		return false;
	}
 
	public int hashCode() {
		return attribut;
	}
}