Probleme mit eigener equals Methode

Brainiac

Bekanntes Mitglied
So ich habe die Klasse Patient, davon speichere ich viele Objekte in einer Liste. Da ich nun verhindern möchte das zweimal das gleiche Objekt in der Liste gespeichert wird habe ich für die Klasse Patient eine equals(Object o) Methode geschrieben, damit ich dann vor dem einfügen Testen kann ob es schon vorhanden ist. Leider fügt er mir das eigentlich gleiche Object (Ich parse das aus der Zwischenablage) immer wieder ein. Ich vermute daher das ich die equals Methode in der Klasse Patient nicht ganz korrekt implementiert habe, vor allem, da mir da Netbeans auch noch Vorschläge zu gemacht hat.

Ich weiß das der _ hat eigentlich nix am Variablen Anfang zu suchen hat, hatte mir das mal für Klassenweite Variablen angewöhnt und arbeite gerade daran das wieder abzustellen. Also nicht dran stören.

Der ID Wert in der Klasse Patient wird erst nach dem einfügen in die DB gesetzt, daher muss er für das equals ignoriert werden. (Wird der interne Zähler der DB).

So hoffe ihr könnt mir helfen:

So hier der Code Ausschnitt vom einfügen:
Java:
private List<Patient> _patientList;
_patientList = new ArrayList<Patient>();

Patient actualPatient = _KHValues.parsePatientFromHTML(patientFromClipboard);
if (!_patientList.contains(actualPatient)) {
    actualPatient.setID(_DBHandler.insertPatient(actualPatient));
    _patientList.add(actualPatient);
    repaint();
}

Und hier die Klasse Patient
Java:
/**
 * @author Brainiac
 */

package de.brainiac.kapihospital.khmanager;

import java.util.Arrays;

public class Patient extends Object {
    private int _ID;
    private String _patientName;
    private int[] _diseases;
    private boolean[] _treatedDiseases;
    private Double _minPrice, _maxPrice;
    
    public Patient(String name, int[] diseasses, Double minPrice, Double maxPrice) {
        _patientName = name;
        _diseases = diseasses;
        _minPrice = minPrice;
        _maxPrice = maxPrice;

        _treatedDiseases = new boolean[_diseases.length];
        for (boolean b : _treatedDiseases) {
            b = false;
        }
    }

    public Patient(int id, String name, int[] diseasses, boolean[] treatedDiseases, Double minPrice, Double maxPrice) {
        _ID = id;
        _patientName = name;
        _diseases = diseasses;
        _treatedDiseases = treatedDiseases;
        _minPrice = minPrice;
        _maxPrice = maxPrice;        
    }

    public int getID() {
        return _ID;
    }

    public void setID(int i) {
        _ID = i;
    }

    public String getName() {
        return _patientName;
    }

    public int[] getDiseases() {
        return _diseases;
    }

    public boolean[] getTreatedDiseases() {
        return _treatedDiseases;
    }

    public int getNumberOfDiseases() {
        int numberOfDiseases = 0;
        for (boolean b : _treatedDiseases) {
            if (!b) {
                numberOfDiseases++;
            }
        }
        return numberOfDiseases;
    }

    public Double getMinPrice() {
        return _minPrice;
    }

    public Double getMaxPrice() {
        return _maxPrice;
    }

    @Override
    public boolean equals(Object o) {
        if (o == null) {
            return false;
        } else if (o.getClass() != getClass()) {
            return false;
        } else if (!((Patient)o).getName().equalsIgnoreCase(_patientName)) {
            return false;
        } else if (Arrays.equals(((Patient)o).getDiseases(), _diseases)) {
            return false;
        } else if (((Patient)o).getMinPrice() != _minPrice) {
            return false;
        } else if (((Patient)o).getMaxPrice() != _maxPrice) {
            return false;
        } else if (((Patient)o).hashCode() != hashCode()) {
            return false;
        }
        return true;
    }

    @Override
    public int hashCode() {
        int hash = 7;
        hash = 79 * hash + (this._patientName != null ? this._patientName.hashCode() : 0);
        hash = 79 * hash + Arrays.hashCode(this._diseases);
        hash = 79 * hash + Arrays.hashCode(this._treatedDiseases);
        hash = 79 * hash + (this._minPrice != null ? this._minPrice.hashCode() : 0);
        hash = 79 * hash + (this._maxPrice != null ? this._maxPrice.hashCode() : 0);
        return hash;
    }
}
 
In Zeile 83 fehlt das "!".
Außerdem ist es sicherer die eigenen Attribute als LValue zu verwenden, falls die Getter des übergebenen Objects null zurückliefern.

Wenn das nicht geholfen hat: einfach Mal einen Breakpoint an den Anfang der Funktion setzen und gucken was passiert.
 
Zudem was Ariol sagte musst du auch equals bei deinem minPrice maxPrice benutzen
ala`
Java:
        } else if (!((Patient)o).getMinPrice().equals(_minPrice)) {
            return false;
        } else if (!((Patient)o).getMaxPrice().equals(_maxPrice)) {

da du in deinen Methoden
Code:
Double
zurück lieferst(also Objekte) und keine
Code:
double
! == -> Referenzvergleich

(warum castest du nicht eig. einmal zu Beginn der equals Methode anstatt jedes mal ? 😀 )
 
Das mit dem Ausrufezeichen hatte ich übersehen, danke. Ist geändert.

Die Double hab ich auch mal in double umgeändert. Keine Ahnung warum ich da Objekte benutz habe.
Das mit dem Cast hatte ich vorher mit einmal casten, aber dann fing Netbeans an mir da Vorschläge für die Methode zu machen und dann hab ich das umgestellt, das die Warnung und Meldungen weg waren, so sollte es aber auch gehen und sieht schöner aus.

Zum Verständniss:
Code:
_patientList.contains(actualPatient)

nimmt nun das actualPatient Object und ruft nacheinander mit allen in der liste befindlichen Objekten die equals Methode von actualPatient auf, oder wird von jedem Element in der Liste die equals Methode aufgerufen und actualPatient übergeben? Nur damit ich beim Debuggen besser verstehe was passiert.

@Ariol was meinst Du mit dem Satz:
Attribute als LValue zu benutzen? Gib mal nen kurzes Bsp. Meinst Du die Notation?
 
Statt
Java:
((Patient)o).getName().equalsIgnoreCase(_patientName)
das hier verwenden:
Java:
!_patientName.equalsIgnoreCase(((Patient)o).getName())


Also etwa so:
Java:
    @Override
    public boolean equals(Object o) {
        if (o == null) {
            return false;
        } 
        if (o.getClass() != getClass()) {
            return false;
        } 

        Patient patient = (Patient)o;

		if (!_patientName.equalsIgnoreCase(patient.getName())) {
			return false;
		}			
        if (!Arrays.equals(_diseases,patient.getDiseases())) {
            return false;
        }
		if (!_minPrice.equals(patient.getMinPrice())) {
            return false;
        } 
		if (!_maxPrice.equals(patient.getMaxPrice())) {
            return false;
        }
		if (patient.hashCode() != hashCode()) {
            return false;
        }
        return true;
    }

Wenn du dann in einem der Getter etwas änderst, so dass null zurückgegeben wird bekommst du so keine NPE.

EDIT: Dadurch dass du mit return abbrichst brauchst du kein else. Ich finde der Code ist so lesbarer ::wink::. Macht keinen Unterschied in der Ausführung. Reine Geschmacksache.
 
Zuletzt bearbeitet:
Danke für eure Hilfen:

Funktioniert nun, aber eine Frage hätte ich noch:
Netbeans hat mir dann die hashCode() Funktion gestrikt. Würde nicht eigentlich nur die Überprüfung der beiden hashWerte ausreichen?

die beiden Funktionen equals und hashCode:
Java:
    @Override
    public boolean equals(Object o) {
        Patient patient = (Patient)o;
        if (o == null) {
            return false;
        }
        if (o.getClass() != getClass()) {
            return false;
        }
        if (!_patientName.equalsIgnoreCase(patient.getName())) {
            return false;
        }
        if (!Arrays.equals(_diseases, patient.getDiseases())) {
            return false;
        }
        if (_minPrice != patient.getMinPrice()) {
            return false;
        }
        if (_maxPrice != patient.getMaxPrice()) {
            return false;
        }
        if (hashCode() != patient.hashCode()) {
            return false;
        }
        return true;
    }

    @Override
    public int hashCode() {
        int hash = 5;
        hash = 47 * hash + (this._patientName != null ? this._patientName.hashCode() : 0);
        hash = 47 * hash + Arrays.hashCode(this._diseases);
        hash = 47 * hash + (int) (Double.doubleToLongBits(this._minPrice) ^ (Double.doubleToLongBits(this._minPrice) >>> 32));
        hash = 47 * hash + (int) (Double.doubleToLongBits(this._maxPrice) ^ (Double.doubleToLongBits(this._maxPrice) >>> 32));
        return hash;
    }
 
die Gleichheit der Hashes sagt was über eine abstrakte Form von Ähnlichkeit von Objekten aus, aber nicht unbedingt etwas über Gleichheit,
es gibt nur x Mill. ints, also verschiedene Hashcodes, aber bereits mehr Strings mit nur 10 Zeichen oder was auch immer

"Hallo" und "jhfhuhfuieefhiuwehwefuih" könnten zufällig denselben Hashcode haben, sind die dann auch gleich?
den Hashcode zu vergleichen kann aus der equals-Methode eigentlich gestrichen werden, wurde das automatisch erzeugt?
 
Der ID Wert in der Klasse Patient wird erst nach dem einfügen in die DB gesetzt, daher muss er für das equals ignoriert werden.
Du beschreibst die Ursache deines Problemes.

Die ID sollte das einzige sein, wonach man Entities unterscheidet, deine equals & hashCode Methoden wären dann auch wirklich simpel 😉
Eine mögliche Lösung wäre, die ID nicht erst von der DB vergeben zu lassen.
 
Nein die Hashwerte zu vergleichen reicht nicht, es gilt zwar das gegeben equals ist wahr für zwei Objekte auch die Hashwerte identisch zu sein haben aber daraus zu schließen das wenn zwei identische Hashwerte vorliegen auch equals() true liefern würde ist nicht richtig. Letzteres "kann" sein "muss" aber nicht.

Apropos leserliche Form das && in Java ist ein "Short-Circuit"-Operator, d.h. sobald er auf ein "false" trifft bricht er die Auswertung ab und gibt false für den Ausdruck zurück ohne sich den Rest anzusehen. Das equals könnte man leserlicher z.b. auch so schreiben:

Java:
    @Override
    public boolean equals(Object o) {
        return o != null 
            && o.getClass() == getClass()
            && equals((Patient)o);
    }
 
    private boolean equals(Patient other){
        return _patientName.equalsIgnoreCase(other._patientName)
            && Arrays.equals(_diseases, other._diseases)
            && _minPrice.equals(other._minPrice)
            && _maxPrice.equals(other._maxPrice)
            && hashCode() == other.hashCode();
    }

PS: Die underscores kannst du lassen, manche schreiben sie auch dahinter. Erstens sieht man die "privaten" member sowieso nicht von aussen und Zweitens spart man sich dann dieses Gefummel mit "this" wenn man die Methodenargumente gleich nennt etc.
 
Zuletzt bearbeitet:
[...]
Die ID sollte das einzige sein, wonach man Entities unterscheidet[...]

Das ist nicht ganz richtig, wenn du z.B. eine Schicht wie Hibernate verwendest benötigst du equals() fürs "dirty-checking", also um zu überprüfen ob sich der Objektzustand geändert hat, die ID aber identisch bleibt/bleiben muss.
 
Zuletzt bearbeitet:
Das ist nicht ganz richtig, wenn du z.B. eine Schicht wie Hibernate verwendest benötigst du equals(), fürs "dirty-checking", sprich ob sich der Objektzustand geändert hat die ID aber identisch bleibt/bleiben muss.
Doch, das ist Grundsätzlich richtig, das ist das entscheidende Merkmal von Entities: Sie unterscheiden sich nicht anhand ihrer Attributwerte, sondern haben eine Identität, ein Hans Müller ist eben eindeutig, aber nicht vom namen her.

"dirty checking" dagegen ist falsch, Hibernate nutzt die id nur in der insertOrUpdate Methode (die man aber cniht wirklich braucht wenn man seine UseCases kennt), dirty checking wird mit entweder mit PropertyChangeSupport umgesetzt, oder mit direktem Attributvergleich, aber die ID darf sich ja nie ändern wenn sie mal gesetzt ist, d.h. dirty checking mit der Id macht gar keinen Sinn und daher macht equals als dirty check keinen Sinn für Entities (für ValueObjects/Embeddables dagegen schon).
 
Doch, das ist Grundsätzlich richtig, das ist das entscheidende Merkmal von Entities: Sie unterscheiden sich nicht anhand ihrer Attributwerte, sondern haben eine Identität, ein Hans Müller ist eben eindeutig, aber nicht vom namen her.

"dirty checking" dagegen ist falsch, Hibernate nutzt die id nur in der insertOrUpdate Methode (die man aber cniht wirklich braucht wenn man seine UseCases kennt), dirty checking wird mit entweder mit PropertyChangeSupport umgesetzt, oder mit direktem Attributvergleich, aber die ID darf sich ja nie ändern wenn sie mal gesetzt ist, d.h. dirty checking mit der Id macht gar keinen Sinn und daher macht equals als dirty check keinen Sinn für Entities (für ValueObjects/Embeddables dagegen schon).

Ich glaube nicht das ich davon gesprochen habe das sich die Id ändert oder irgendwie in einem dirty-check verarbeitet wird. Spontan fällt mir zu dem Thema, weshalb ein ID-Vergleich vll. nicht die beste Idee ist, Kapitel 9 aus Java Persistence with Hibernate 2nd ed. ein. Da findest du auch Inspiration über die Ursachen weshalb man vll. doch ein equals benötigt wenn man mit Hibernate arbeitet.

Und die Use-Cases bei mir vor Ort verlangen ein sauber implementiertes equals, da es zu den sonderbarsten Effekten kam wenn die equals-Methoden nicht richtig implementiert wurden.
 
Ich glaube nicht das ich davon gesprochen habe das sich die Id ändert oder irgendwie in einem dirty-check verarbeitet wird. Spontan fällt mir zu dem Thema, weshalb ein ID-Vergleich vll. nicht die beste Idee ist Kapitel 9 aus Java Persistence with Hibernate 2nd ed. ein. Da findest du auch Inspiration über die Ursachen weshalb man vll. doch ein equals benötigt wenn man mit Hibernate arbeitet.
equals benötigt man immer, nicht nur mit Hibernate, hab nix anderes behauptet, aber bei Entitities ist eben die ID auschlaggegebend.
Warum Entites Ids brauchen, selbst bevor sie in der Db gespeichert werden, findest du in u.a. Domain Driven Design von Eric Evans.

Wenn man das ORM die ID vergeben lässt, ist man abhängig davon, ob die Entities transient ist oder schon persistiert sind, dann kann man nur entweder alles speichern (und ggf. wieder löschen) bevor man damit arbeitet, oder man kann plötzlich nicht mehr Java Standard Collections und Maps verwenden.

Und die Use-Cases bei mir vor Ort verlangen ein sauber implementiertes equals, da es zu den sonderbarsten Effekten kam wenn die equals-Methoden nicht richtig implementiert wurden.
Eben, wirklich sauber heisst aber :
1. Die ID wird bei Objekterzeugung vergeben
2. equals bezieht nur auf die Id, sonst nix

Die Ids vom ORM vergeben zu lassen funktioniert eben nur bei einfachen CRUD Anwendungen wenn man die Einschränkungen beachtet, ist keine saubere Lösung, sondern fragil und eher als "Q&D" einzuordnen.
 
Ich glaube wir reden aneinander vorbei. Mein Beispiel war explizit auf Hibernate gemünzt und bei Hibernate läuft es, meines Erachtens nach so, dass wenn ein Objekt innerhalb einer "unit-of-work" geändert wird Hibernate "automatisch" das Objekt in der DB aktualisiert wenn es eine Änderung bemerkt und um zu sehen ob der persistente Zustand aktualisiert werden muss wird ein per-value-Vergleich durchgeführt und dazu werden die equals-Methoden verwendet. Und wenn man dort nur einen "ID" check macht wird schlimmstensfalls gesagt, es ist das selbe Objekt ohne Änderungen und als Folge wird der persistente Zustand in der DB nicht aktualisiert.
 
Zuletzt bearbeitet:
Ich glaube wir reden aneinander vorbei. Mein Beispiel war explizit auf Hibernate gemünzt und bei Hibernate läuft es, meines Erachtens nach so, dass wenn ein Objekt innerhalb einer "unit-of-work" geändert wird Hibernate "automatisch" das Objekt in der DB aktualisiert wenn es eine Änderung bemerkt und um zu sehen ob der persistente Zustand aktualisiert werden muss wird ein per-value-Vergleich durchgeführt und dazu werden die equals-Methoden verwendet. Und wenn man dort nur einen "ID" check macht wird schlimmstensfalls gesagt, es ist das selbe Objekt ohne Änderungen und als Folge wird der persistente Zustand in der DB nicht aktualisiert.
Wie gesagt, m.E. hast du Hibernate falsch verstanden, equals wird _nicht_ für den dirty check genutzt, sondern die Attribute einzeln verglichen.

EclipseLink zB. nutzt den PropertyChangeSupport, läuft auf dasselbe hinaus: haben sich Attribute geändert oder nicht, equals() hat damit rein gar nix zu tun.

Nur die ID in equals bei Entities zu nutzen ist ein übliches verfahren, das aber nicht sauber funktioniert, wenn die Id erst nach dem persistieren vergeben wird.
 
Mhm, dann werde ich in der Richtung mal nach Literatur suchen, falls dir spontan welche einfallen die da etwas mehr das technische Detail beschreiben immer her damit.
Bei Google findet man was, aber alle Ergebnisse laufen darauf hinaus, dass die Attribute verglichen werden.

Wäre ja auch sehr "einschränkend" (diplomatisch ausgedrückt) wenn equals immer nur alle Attribute vergleichen würde, das ist ValueObject Semantik, nicht die von Entities.
 
Ich sag mal allen Danke! Freut mich das ich euch mit einer einfachen Frage in eine so rege Diskussion verwickeln konnte und noch einige andere was lernen konnten.
 
Ich sag mal allen Danke! Freut mich das ich euch mit einer einfachen Frage in eine so rege Diskussion verwickeln konnte und noch einige andere was lernen konnten.
Ist dir denn nun klar warum es keine gute Idee ist die Attribute einer Entity in equals zu vergleichen?

Was passiert denn, wenn du 2 oder mehr Patienten hast mit demselben namen, min-/maxpreis etc.?
Echte Krankenhaus SW darf so nicht arbeiten...
 

Zurück
Oben