Tutorial equals und hashCode überschreiben

[neurox]

Hallo allerseits,

ich habe ein kleines Tutorial zum Thema equals und hashCode geschrieben:

http://blog.buhbuhbuh.de

Wer sich dafür interessiert, ist herzlich eingeladen mal vorbei zu schauen. Auch für konstruktive Kritik bin ich immer dankbar.

Viele Grüße
neurox

 

[SlaterB]

ich wäre froh, nie
if (xy) befehl;
oder
else befehl;
lesen zu müssen

Klammern, Klammern, Klammern, gerade für die Anfänger

----

zwei Produkte mit gleicher artikelNr, gleichem preis, aber unterschiedlicher produkteBezeichnung
würden bei dir equal sein, aber unterschiedlichen hashCode() liefert -> aua

falls produkteBezeichnung immer von artikelNr abhängig sein soll, (was du wohl kaum garantieren kannst), dann brauchst du es in der hashCode-Berechnung gar nicht

edit:
zudem: produkteBezeichnung != artikelBezeichnung

 

[neurox]

zwei Produkte mit gleicher artikelNr, gleichem preis, aber unterschiedlicher produkteBezeichnung
würden bei dir equal sein, aber unterschiedlichen hashCode() liefert -> aua

In dem Fall hast Du wohl nicht genau gelesen. Ich habe davor ja extra geschrieben, dass ich das Objekt noch mal erweitert habe, damit man sieht, wie man aus einen String auch noch einen hashCode heraus bekommt und nicht nur aus int und BigDecimal. Aber vielleicht ist das auch etwas missverständlich.

Aber mit den Klammer hast Du schon recht. Ich werde das noch mal etwas überarbeiten. Dann stehen da zwar etwas mehr Zeilen, aber vermutlich ist es dann noch leserlicher.

Grüße
neurox

 

[maki]

Hier meine Kritikpunkte:
Zwei BigDecimals mit == zu vergleichen ist eig. nicht korrekt (wie eig. bei allen Objekten)

Würde auch vermeiden, alles in einem if/else Block zu schreiben, Boch macht das in seinem Buch auch anders

 

[neurox]

Hier meine Kritikpunkte:
Zwei BigDecimals mit == zu vergleichen ist eig. nicht korrekt (wie eig. bei allen Objekten)

... das ist sogar ganz und gar nicht korrekt. Habe ich übersehen. Werde ich auch noch ändern.

Würde auch vermeiden, alles in einem if/else Block zu schreiben, Boch macht das in seinem Buch auch anders

Ich sehe schon, das Buch ist beliebt ;-)

 

[maki]

Ich sehe schon, das Buch ist beliebt

Ja, sehr, auch hier.
Stimme zwar nicht mit allem darin überein (wozu braucht jede serialisierbare Klaase eine UUID? Meist will man gar nicht zwischen verschiedenen Versionen serialisieren), würde es aber jedem Java Entwickler empfhelen, nicht nur den angehenden.

 

[faetzminator]

Klammern, Klammern, Klammern, gerade für die Anfänger

Hier haben wir aber ein unnötiges else case vorliegen.

if (a) {
    return b;
} else {
    return c;
}

finde ich leserlicher mit

if (a) {
    return b;
}
return c;

noch schöner fänd ich in diesem Beispiel einen check auf !instanceof und dann ein return false.

 

[neurox]

Ja, sehr, auch hier.
Stimme zwar nicht mit allem darin überein (wozu braucht jede serialisierbare Klaase eine UUID? Meist will man gar nicht zwischen verschiedenen Versionen serialisieren), würde es aber jedem Java Entwickler empfhelen, nicht nur den angehenden.

... oder weshalb man in gar keinen Fall Exemptions in der Geschäftslogik verwenden darf. Wir hatten das ja vor ein paar Tagen man am Beispiel von NumberFormatExceptions diskutiert. Er wäre ganz klar dagegen, allerdings habe ich da nirgends im Netz oder in der Java API eine besser Lösung gefunden.

Meint ihr, es macht Sinn solche Tuts hier einzustellen? Ich habe die mal (auch für mich selbst) geschrieben, weil ich immer wieder auf das Thema komme.

 

[neurox]

Soo, nun habe ich das ganze noch mal überarbeitet und eure Kommentare berücksichtigt.

Des weiteren habe ich den Code auch noch sehr ausführlich erklärt, so dass es nun auch einsteigertauglich sein sollte.

War ein bisschen Arbeit, ich hoffe es hilft dem einen oder anderen.

Grüße
neurox

 

[Spacerat]

Da musst du wohl noch mal bei:

@Override
public boolean equals(Object obj)
{
  if(this == obj) { // kurz und buendig
    return true;
  }
  if(!(obj instanceof Produkt)) {
    return false;
  }
  Produkt p = (Produkt) obj;
  // artikelNr ist primitiv und kann deswegen nicht null sein. Nullcheck eruebrigt sich.
  boolean rc = artikelNr != produkt.artikelNr;
  // equalsIgnoreCase missachtet inhaltliche Gleichheit und sollte deswegen nicht unbedingt verwendet werden.
  // Zustand von artikelBezeichnung und preis haengen von den Implementationen der Set-Methoden ab. Nullchecks erforderlich.
  rc &= artikelBezeichnung == produkt.artikelBezeichnung
            || (artikelBezeichnung != null && artikelBezeichnung.equals(produkt.artikelbezeichnung));
  rc &= preis == produkt.preis || (preis != null && preis.equals(produkt.preis);
  return rc;
}

 

[eRaaaa]

Da musst du wohl noch mal bei:

@Override
public boolean equals(Object obj)
{
  if(this == obj) { // kurz und buendig
    return true;
  }
  if(!(obj instanceof Produkt)) {
    return false;
  }
  Produkt p = (Produkt) obj;
  // artikelNr ist primitiv und kann deswegen nicht null sein. Nullcheck eruebrigt sich.
  boolean rc = artikelNr != produkt.artikelNr;
  // equalsIgnoreCase missachtet inhaltliche Gleichheit und sollte deswegen nicht unbedingt verwendet werden.
  // Zustand von artikelBezeichnung und preis haengen von den Implementationen der Set-Methoden ab. Nullchecks erforderlich.
  rc &= artikelBezeichnung == produkt.artikelBezeichnung
            || (artikelBezeichnung != null && artikelBezeichnung.equals(produkt.artikelbezeichnung));
  rc &= preis == produkt.preis || (preis != null && preis.equals(produkt.preis);
  return rc;
}

du meinst wohl eher

//Produkt p ....
Produkt produkt = (Produkt) obj;

man könnte vllt auch noch

if (obj == null) {
			return false;
		}

vor die ganzen checks machen, die würde man sich dann nämlich sparen im falle von null :bae: - ich geh nu pennen, gn8 an alle

 

[Spacerat]

du meinst wohl eher

//Produkt p ....
Produkt produkt = (Produkt) obj;

Ja, mein ich.

man könnte vllt auch noch

if (obj == null) {
			return false;
		}

vor die ganzen checks machen, die würde man sich dann nämlich sparen im falle von null

...Hab' mir sagen lassen, dass das durch "instanceof" mit abgedeckt wird. An den nötigen Nullchecks der Instanzvariablen "artikelBezeichnung" und "preis" ändert das allerdings nichts.

 

[eRaaaa]

Hab' mir sagen lassen, dass das durch "instanceof" mit abgedeckt wird.

oh, stimmt ! dann nehm ich alles zurück (eclipse auto generate hauts auch rein ;( )

nach der suche wie ich das umschreiben kann, bin ich auf folgenden beitrag gestoßen:

EclipseZone - Generate incorrect equals methods with ...

verwirrt mich jetzt irgendwie wie macht mans denn nu richtig ???:L
gerade auch weil er ja auch das buch anspricht, von dem hier die rede ist.

nu geh ich aber wirklich ins bett, hoffe dass mich bis zu mnachmittag jemand belehrt

 

[The_S]

Wenn du magst, kannst du ja auch mal hier einen Blick rein werfen :

Java Blog Buch : 04.03.11 Besondere Methoden (equals, hashCode und toString)

Außerdem werden (habs mal grob überflogen) die produktBezeichungen nicht verglichen (equals wäre hier angebracht) und die preise falsch (du machst das mit ==, bei einem BigDecimal verwendet man aber compareTo).

Nachtrag: OK, produktBezeichnung wird nicht verglichen, weil du ja die artikelNummer vergleichst. Sorry, hab ich überlesen.

 

[mvitz]

Für das vergleichen der Attribute kann man, wenn man faul ist auch: org.apache.commons.lang.builder (Commons Lang 2.4 API) benutzen.

Weiterhin würde mich jetzt auch nochmal interessieren, was nun beim checken der "Objektart" das richtige ist.

Bisher habe ich das immer wie in "Effective Java" gemacht:

public class A {
  public boolean equals(Object obj) {
    if (obj == this) {
      return true;
    }
    if (!(obj instanceof A)) {
      return false
    }
    A other = (A) obj;
    ...
}

 

[The_S]

Richtig ist alles, was den equals-contract erfüllt

Object (Java 2 Platform SE v1.4.2)#equals

Nachtrag: Da ist wohl was mit dem Link schiefgegangen.

 

[byte]

Am effektivsten gehts imo so: Building equals(), hashCode(), compareTo() and toString() with ease | Benjamin Winterberg

 

[SlaterB]

hashCode muss wirklich nicht alle Attribute beinhalten,
von beispielsweise
private String firstName;
private String lastName;
private Date birthday;
private String street;
private String city;
private String zipcode;
sollte firstName + lastName eindeutig genug sein, im Extremfall noch das Datum,
durch den Rest wirds auch nicht besser

 

[byte]

Lass die Datenbestände geeignet groß werden und Du hast unrecht.

 

[Marco13]

Im Gegenteil: Der Sinn von hashCode ist ja auch, dass er schnell ausgerechnet werden kann. Wenn man sich bei der Berechnung auf Fields beschränken kann, die immutable sind, kann es sogar Sinn machen, eine "private int hashCode" anzulegen, die schon im Konstruktor berechnet wird - und bei Personen sollte das eigentlich der Fall sein ... man ändert seinen Namen nicht 10 mal am Tag, und das Geburtsdatum ändert sich höchstens 4 mal

EDIT: @byte: hashCode muss nicht eindeutig sein, und ist es meistens auch nicht. Auch ein [c]public int hashCode() { return 0; }[/c] wäre "richtig". Blöd, aber richtig.

 

[SlaterB]

wenn die Datenmenge groß ist gibts sowieso Doppelte, allein schon zufällig,
zufällige wären minimal besser, bei equals leichter zu trennen, aber lohnen nicht den Aufwand

 

[Spacerat]

Richtig ist alles, was den equals-contract erfüllt

Schon klar. Aber was hat der Autor des von eraaaa verlinkten Beitrag an "instanceof" auszusetzen (synchron, asynchron oder was der da will). Das einzige was ich mir dabei vorstellen kann, ist, nicht den direkten Klassenvergleich wie im Blog von The_S zu verwenden sondern etwa etwas wie

getClass().isInstance(obj);

was das dynamische Äquivalent zu "instanceof" darstellt. Allerdings bräuchte man diese Dynamic ja nur bei Generics und da stellt sich die Frage, ob die Methode überhaupt zwischen verschiedenen Typ-Klassen unterscheiden kann. In welchen Fällen wäre den der Vertrag beim Verwenden von "instanceof" nicht mehr erfüllt?

 

[maki]

Denke [c]Class.isInstance[/c] würde nicht funktionieren wenn es sich um Proxies handelt (zB. in Verbindung mit Hibernate), gilt aus für getClass.

[c]instanceOf[/c] hat den Nachteil, dass es u. U. nicht symetrisch funzt, Beispiel:
Klasse B erbt von A.
Klasse A hat ein Attribut key, das in equals (& hascode) als einziges verwendet wird.
Objekt a und b haben denselben Wert in Key.
a.equals() nutzt instanceOf, um herauszufinden, ob b auch ein A ist -> ist es, equals ergibt true.
b.equals nutzt instanceOf, um zu prüfen ob a auch ein B ist -> ist es nicht, equals ergibt false.

 

[SlaterB]

instanceOf und Class.isInstance sind äquivalent, Subklassen sind kein Problem

das andere Problem ist gegeben, aber liegt doch nicht an instanceOf,
wozu sollte eine Subklasse equals überschreiben, wenn sie nicht auf zusätzliche Eigenschaften prüft?

 

[Spacerat]

instanceOf und Class.isInstance sind äquivalent, Subklassen sind kein Problem

das andere Problem ist gegeben, aber liegt doch nicht an instanceOf,
wozu sollte eine Subklasse equals überschreiben, wenn sie nicht auf zusätzliche Eigenschaften prüft?

Es ging mir nicht um Sub-Klassen, sondern um Typ-Klassen bei Generics. Wenn "<Class>.isInstance()" da unterscheidet (z.B. List<String> != List<Integer>), hätte es einen riesigen Vorteil. Das mit der Symetrie (genau... das war's und nicht "a-/synchron") wird mir iwie nicht Klar (sch... Pneumatik. Ich steh' auf'm Schlauch ). Das hängt doch von der Hierarchie ab und (auch in makis Beispiel) B ist stets ein A aber A niemals ein B. Ob das nun mit "<Class>.isInstance()" oder "instanceof" geprüft wird.

 

[SlaterB]

mit dir redet doch keiner

 

[maki]

instanceOf und Class.isInstance sind äquivalent, Subklassen sind kein Problem

das andere Problem ist gegeben, aber liegt doch nicht an instanceOf,
wozu sollte eine Subklasse equals überschreiben, wenn sie nicht auf zusätzliche Eigenschaften prüft?

Auch wieder richtig... dann hab ich den Blog auch nciht verstanden ???:L

 

[byte]

Im Gegenteil: Der Sinn von hashCode ist ja auch, dass er schnell ausgerechnet werden kann. Wenn man sich bei der Berechnung auf Fields beschränken kann, die immutable sind, kann es sogar Sinn machen, eine "private int hashCode" anzulegen, die schon im Konstruktor berechnet wird - und bei Personen sollte das eigentlich der Fall sein ... man ändert seinen Namen nicht 10 mal am Tag, und das Geburtsdatum ändert sich höchstens 4 mal

EDIT: @byte: hashCode muss nicht eindeutig sein, und ist es meistens auch nicht. Auch ein [c]public int hashCode() { return 0; }[/c] wäre "richtig". Blöd, aber richtig.

Ob ich den Hashcode nun über 2, 3 oder 5 primitiven Member berechne, ist für mich ehrlich gesagt ziemlich irrelevant. Solche Micro-Optimierungen bringen in der Praxis selten spürbare Unterschiede.
Primär muss der Hashcode möglichst disjunkt sein. Wenn ich 300.000 Personen in ein HashSet schmeisse und der Hash nur auf Vor- und Nachnahme geht oder gar immer 0 liefert, dann ist das Ergebnis einfach ungenügend.
Und wenn die Performance der Hashcode Bildung dann doch mal relevant ist, kann man den Hashcode (auch bei Mutables) einfach cachen.

Edit: Im übrigen gings mir bei dem Link eher um den Hinweis auf die Apache Commons.

 

[SlaterB]

wie kann das Einsparen paar Member kaum spürbar sein, dann aber das Ergebnis einfach ungenügend

beides ist im Microbereich, 100 Doppelte von 300.000 machen nicht mehr Arbeit als 300.000x 3 Felder extra zu berechnen,
im Zweifel muss man es ausprobieren bzw. ist beides egal, war ja nur generell angemerkt

 

[neurox]

Ich denke mal die sinnvollste Lösung, um HashCode schnell berechnen zu können, ist ein Caching. In dem viel zitierten Buch von Joshua Bloch hat er auch vorgeschlagen, das bei der Initialisierung zu berechnen und dann den gecachten Wert aus zu geben. In dem Fall muss man dann nur schauen, was passiert, wenn sich das Objekt ändert.

Ich habe diese ganzen Dinge in dem Tutorial jetzt nicht mehr behandelt, weil es eben für Anfänger verständlich sein soll. Ich denke mal die ganzen Gesichtspunkte hinsichtlich der Performance wird jeder nur im Laufe der Zeit durch praktische Erfahrungen kennenlernen.

Es kommt dann auch zu sehr auf den Anwendungsfall an, als dass man nun allgemeingültig ein Tuning erklären könnte. Das Objekt, was ich hier angelegt habe, taugt ja ohnehin nicht um es irgendwo als echtes Geschäftsobjekt zu verwenden.

 

[byte]

wie kann das Einsparen paar Member kaum spürbar sein, dann aber das Ergebnis einfach ungenügend

beides ist im Microbereich, 100 Doppelte von 300.000 machen nicht mehr Arbeit als 300.000x 3 Felder extra zu berechnen,
im Zweifel muss man es ausprobieren bzw. ist beides egal, war ja nur generell angemerkt

Wenn ich 300.000 fachlich unterschiedliche Personen in ein HashSet schmeisse, dann erwarte ich, dass sich danach 300.000 und nicht 299.900 Personen im HashSet befinden. Letzteres erzeugt falsches Systemverhalten und das ist für mich ungenügend. Etwaige Performanceprobleme werden erst dann gelöst, wenn sie auftreten und nicht schon vorher.

 

[SlaterB]

selbst mit hashCode 0 sind 300.000 Personen drin, wenn equals funktioniert,
die Anzahl ist nicht betroffen, nur der Aufwand für contains() usw. erhöht sich
?

> Etwaige Performanceprobleme werden erst dann gelöst, wenn sie auftreten und nicht schon vorher.

demnach immer hashCode 0, Performance egal?

 

[bygones]

Ich habe diese ganzen Dinge in dem Tutorial jetzt nicht mehr behandelt, weil es eben für Anfänger verständlich sein soll. Ich denke mal die ganzen Gesichtspunkte hinsichtlich der Performance wird jeder nur im Laufe der Zeit durch praktische Erfahrungen kennenlernen.

full ack.

als fahranfaenger eines autos will ich wissen wie ich fahr und wie ich brems... wie ich bei 180kmh um die Kurve bieg oder wie ich mit der Handbremse schleudernd parken will ist einfach uninteressant.

 

[The_S]

wie ich bei 180kmh um die Kurve bieg oder wie ich mit der Handbremse schleudernd parken will ist einfach uninteressant.

Findest du? Das würde den Unterricht doch sehr viel interessanter machen