equals & compareTo

[max5432]

Bin etwas unsicher bezüglich Implementierung von equals und compareTo in einer Vater-Kind-Klassenhierarchie. Wäre froh, wenn mir jemand helfen würde.

Klasse Figur:

public class Figur implements Comparable<Figur>
{
	private int id;
	private int laenge;
	private int breite;
	
	// ... 
	
	public boolean equals(Object obj)
	{
		if (this == obj)
			return true;
		else if (obj == null)
		{
			return false;
		}
		else
		{
			final Figur f = (Figur)obj;
			
			return id == f.id;
		}
	}
	
	public int compareTo(Figur figur)
	{
		if (this == figur)
		{
			return 0;
		}
		else if (figur.getClass() != getClass())
		{
			throw new ClassCastException(); 
		}
		else
		{
			if (laenge == figur.laenge)
			{
				return breite - figur.breite; 
			}
			else
			{
				return laenge - figur.laenge; 
			}
		}
	}
}

Und die Klasse Body:

public class Body extends Figur
{
	private int hoehe;
	
	
	// ... 
	
	
	public int compareTo(Figur figur)
	{
		if (this == figur)
		{
			return 0;
		}
		else if (figur.getClass() != getClass())
		{
			throw new ClassCastException(); 
		}
		else
		{
			Body b = (Body)figur; 
			
			if (super.compareTo(figur) == 0)
			{
				return hoehe - b.hoehe; 
			}
			else
			{
				return super.compareTo(figur); 
			}
		}
	}
}

Frage 1:
Das Prüfen nach Gleichheit wird in beiden Methoden unterschiedlich implementiert: in der Methode equals wird nur die Instanzvariable id geprüft, während in der Methode compareTo andere Attribute (laenge & breite bzw. laenge, breite und hoehe) berücksichtigt werden. Wird so etwas (wenn auch nur in seltenen Situationen) praktiziert? Gebe es sonst ein gutes Beispiel, in dem eine solche (unterschiedliche) Implementierung sinnvol eingesetzt wird?

Frage 2:
Die Implementierung der Methode compareTo in der Unterklasse Body ist syntaktisch korrekt (semantisch glaube auch). Nun, macht man das so? Oder, gebe es andere Möglichkeiten, die evtl. besser sind?

Frage 3:
Anstatt

if (figur.getClass() != getClass())

könnte man auch den Operator instance of einsetzen:

iif (!(figur instanceof Body))

Wo liegt der Unterschied? Ist die getClass-Variante effizienter? Den, in vielen Tutorials wird diese Variante eingesetzt. Was ist der Grund dafür?

Frage 4:
Folgende Zeile interessiert mich noch:

final Figur f = (Figur)obj;

Gehe ich richtig in der Annahme, dass final nur dann nötig ist, wenn die Gefahr besteht, dass das übergebene Objekt geändert wird, bevor die Methode equals abgearbeitet wird? Wenn diese Gefahr nicht besteht, könnte man auf final auch verzichten, wie dies in der folgenden Zeile der Fall ist:

Body b = (Body)figur;

Oder, ist dies doch anders?

Hinweis:
Die Methode hashCode habe ich hier bewust nicht überschrieben bzw. rausgelassen, obwohl die Methode equals überschrieben wurde.

Danke

 

[SlaterB]

Frage 2:
wenn die compareTo in Figur so bleibt (siehe meine Anmerkung zu Frage 3),
könntest du in Figur auf den Klassentest verzichten und nach dem == Vergleich erstmal super.compareTo aufrufen,
dann fliegt da eben die ClassCastException, wenn nicht, kannst du danach unbesorgt auf Body casten

zweimal super.compareTo aufzurufen ist unnötig, speichere den Rückgabewert doch in einer int-Variable

allgemein: wenn ein if-Fall ein return liefert oder eine Exception wirft, könnte man sich sparen, den Code dahinter in ein else zu packen, es kann normal weitergehen

Frage 3:
stell dir einen Aufruf
figur.compareTo(body);
vor,

die Klassen sind unterschiedlich, bei dir fliegt eine ClassCastException,

würde in Figur.compareTo dagegen instanceof getestet, dann könnte man den Vergleich durchführen,
denn Body ist eine Instanz von Figur, läßt sich auf Figur casten,
andersrum ginge es natürlich nicht

Frage 4:
final ist hier unnötig, der Parameter kann sich nicht ändern, es kann kein anderes Objekt irgendwo her gezaubert kommen,
der interne Zustand könnte sich nebenläufig ändern, ja, aber dagegen hilft final auch nicht

andererseits kann man auch praktisch immer final verwenden, nur dann nicht wenn es anders sein muss,
ist ein Hinweis, dass der Variablen nie was anderes zugewiesen wird,
das hilft allgemein dem Programmierer beim Verstehen, wenn man denn dran gewöhnt ist (eher unnötig bei nur 2-3 Zeilen Code)
sowie dem Compiler beim Optimieren der Ausführung (auch eher wenig in dieser Situation)


edit:
Frage 1:
in compareTo nach der Id zu sortieren würde zu einer anderen Reihenfolge führen,
ob man das will oder nicht muss man selber wissen,
man könnte aber wie in equals die Id anschauen oder gleich Equals aufrufen und dann 0 zurückgeben,
einen equals-Aufruf sieht man häufig in compareTo

ob nun equals alle Attribute oder die Id anschauen soll ist ne größere Frage,
da soll mal wer anders was zu schreiben

 

[max5432]

Danke erstmal für die Antwort.

Frage 2:

wenn die compareTo in Figur so bleibt (siehe meine Anmerkung zu Frage 3),
könntest du in Figur auf den Klassentest verzichten und nach dem == Vergleich erstmal super.compareTo aufrufen,
dann fliegt da eben die ClassCastException, wenn nicht, kannst du danach unbesorgt auf Body casten

Heisst (falls ich es richtig verstanden habe), die Body.compareTo sollte wie folgt aussehen:

public class Body extends Figur
{
	private int hoehe;
	
	
	// ... 
	
	
	public int compareTo(Figur figur)
	{
		if (this == figur)
		{
			return 0;
		}
		
		int res = super.compareTo(figur); 

		Body b = (Body)figur; 

		if (res == 0)
		{
			return hoehe - b.hoehe; 
		}
		
		return res; 
	}
}

Frage 3:

Die Figur.compareTo sollte wie folgt aussehen:

public int compareTo(Figur figur)
{
	if (this == figur)
	{
		return 0;
	}

	if (!(figur instanceof Figur))
	{
		throw new ClassCastException(); 
	}

	if (laenge == figur.laenge)
	{
		return breite - figur.breite; 
	}

	return laenge - figur.laenge; 
}

War es so gemeint? Oder, habe ich es nicht ganz richtig verstanden?

Danke

 

[SlaterB]

wenn du in Figur nun instanceof verwendest, dann musst du in Body auch instanceof verwenden,
sonst führt body.compareTo(figur) zu einer ClassCastException, da der Test in super, also in Figur, nicht meckern wird,

wie du es haben möchtest musst du entscheiden, beides denkbar,


--------

 public int compareTo(Figur figur)
    {
        if (this == figur)
        {
            return 0;
        }
        
        int res = super.compareTo(figur); 
 
        Body b = (Body)figur; 
 
        if (res == 0)
        {
            return hoehe - b.hoehe; 
        }
        
        return res; 
    }

könnte man als schlecht aufgebaut sehen, eine Alternative wäre

 public int compareTo(Figur figur)
    {
        if (this == figur)
        {
            return 0;
        }
        
        int res = super.compareTo(figur); 
        if (res != 0)
        {
            return res; 
        }
 
        Body b = (Body)figur; 
        return hoehe - b.hoehe; 
    }

aber das sind dann nur noch Kleinigkeiten

 

[Marco13]

Habe den Code jetzt nicht komplett nachvollzogen, und auch wenn ich davon ausgehe, dass SlaterB das bei seinen Erläuterungen implizit berücksichtigt hat, hier nochmal der explizite Hinweis auf die Bedingung, die an "compareTo" gestellt wird:
Comparable (Java 2 Platform SE v1.4.2)

Was dort unter dem Stichwort "consistent with equals" steht, sollte man barücksichtigen - sonst kracht's spätestens in einer TreeMap...

 

[max5432]

Sehe immer noch nicht ganz durch!

SlaberB:
wenn du in Figur nun instanceof verwendest, dann musst du in Body auch instanceof verwenden, sonst führt body.compareTo(figur) zu einer ClassCastException, da der Test in super, also in Figur, nicht meckern wird,

Heisst, dass in der Body.compareTo folgendes stehen muss:

if (!(figur instanceof Figur))
{
    throw new ClassCastException();
}

Denn, wenn ich

if (!(figur instanceof Body))
{
    throw new ClassCastException();
}

schreibe (irgendwie naheliegender, da in der Klasse Body), klappt ein

body.compareTo(figur)

nicht. Die Schlussfolgerung wäre, dass beim Vergleichen von Instanzen einer Super- und einer Subklasse nur das gemeinsame verglichen wird: sinnlos ist dies überhaupt nicht. Andererseits würde ich aber auch erwarten, dass ein

body.compareTo(figur)

eine Ausnahme wirft, da diese zwei Instanzen i.d.R. unterschiedlich aufgebaut und somit "nicht restlos vergleichbar" sind.

Was heisst das jetzt?

Variante 1:
Sollte das Vergleichen von Super- und Subklasse-Instanzen auf das Vergleichen von gemeinsamen Elementen reduziert werden, muss man in der Subklasse.compareTo mit param instanceof Superklasse arbeitet.

public class Body extends Figur
{

    private int hoehe;

    // ... 

    public int compareTo(Figur figur)
    {
        if (this == figur)
        {
            return 0;
        }

        if (!(figur instanceof Figur))  // !!!
        {
            throw new ClassCastException(); 
        }

        int res = super.compareTo(figur);

        if (res != 0)
        {
            Body b = (Body) figur;
            return hoehe - b.hoehe;
        }

        return res;
    }
}

Variante 2:
Sollte das Vergleichen von Super- und Subklasse-Instanzen verhindert werden, wirft man eine ClassCasstException, indem man in der Subklasse.compareTo mit param instanceof Subklasse oder param.getClass() != getClass() arbeitet.

public class Body extends Figur
{

    private int hoehe;

    // ... 

    public int compareTo(Figur figur)
    {
        if (this == figur)
        {
            return 0;
        }

        if (!(figur instanceof Body))  // !!!
        {
            throw new ClassCastException(); 
        }

        int res = super.compareTo(figur);

        if (res != 0)
        {
            Body b = (Body) figur;
            return hoehe - b.hoehe;
        }

        return res;
    }
}

Interessant ist, dass im Falle, dass ich auf das Prüfen vom Typ des Parameters verzichte (wie SlaberB schon geschrieben), die Variante 1 (automatisch) gewählt wird: das Vergleichen reduziert sich auf das Gemeinsame. So kann die Variante 1 auf das folgende reduziert werden:

public class Body extends Figur
{

    private int hoehe;

    // ... 

    public int compareTo(Figur figur)
    {
        if (this == figur)
        {
            return 0;
        }

        int res = super.compareTo(figur);

        if (res != 0)
        {
            Body b = (Body) figur;
            return hoehe - b.hoehe;
        }

        return res;
    }
}

Wie wird diese Prüfung (im Hintergrund) durchgeführt? Bzw. steht dies irgendwo als vorgegeben (contract), dass das Vergleichen auf das Gemeisname reduziert werden soll / muss? Oder, wird hier "nach gesundem Menschenverstand" gehandelt?

wie du es haben möchtest musst du entscheiden, beides denkbar,

Was meinst du jetzt mit "beides"? Eine ist sicher mit instanceof sowohl in der Super- als auch in der Subklasse. Und was ist das zweite? Das weglassen der Typ-Prüfung?

Danke im Voraus.

 

[SlaterB]

mit beides meinte ich,

1.
Figur vergleicht nur den Figur-Anteil, egal ob das andere Objekt nun ein Body ist oder Figur
2.
überall immer erst auf identische Klassen prüfen, dann gehts weiter

ich tendierte in meinen Postings hier bisher zu 1.,
aber wenn man noch Nähe zu equals bedenkt und die Symmetrie (a.compareTo(b), b.compareTo(a)),
siehe auch Marco13s Link, dann ist 2. vielleicht die saubere Lösung,


und nochmal bezogen auf die Implementierung:
wenn 2., dann reicht getClass() == other.getClass()-Vergleich in Figur, Body kann das beim super-Aufruf mitverwenden,

wenn dagegen instanceof, also Variante 1, dann braucht Figur das, aber Body auch,
dass Body ein Figur-Objekt als Parameter akzeptiert ist wohl extrem exotisch und auszuschließen,
deshalb wäre 1. nicht besonders symmetrisch und nicht gerade an equals angeleht:
figur.compareTo(body) liefert <,= oder >
body.compareTo(figur) liefert ClassCastException

 

[max5432]

Vielen Dank.

Nun, ich bin immer noch nicht am Ziel und würde gerne eine rekursive Datenstruktur realisieren, bei der sowohl die equals als auch die compareTo entsprechend implementiert werden sollen.

Angenommen, ich brauche einen FigurContainer (eine grosse Figur, in der mehrere kleinere Figuren enthalten sind usw.). Die Klasse FigurContainer ist allerdings eine Subkalsse der Klasse Figur und hat zusätzlich eine Collection, in der die Figur-Objekte verwaltet werden, und die Kapazität (die maximale Anzahl Figur-Elemente im Container). Ich habe diese Klasse wie folgt implementiert:

/* Created on 24.08.2009 */

public class FigurContainer extends Figur
{
    private Collection<Figur> figurList; 
    private int capacity; 
    
    public FigurContainer(int laenge, int breite, int capacity)
    {
        super(laenge, breite);
        this.capacity = capacity;
        
        figurList = new ArrayList<Figur>(capacity);
    }

    public boolean add(Figur figur)
    {
        if (capacity > figurList.size())
        {
            return figurList.add(figur); 
        }
        
        return false;
    }
    
    public boolean remove(Figur figur)
    {
        if (figurList.contains(figur))
        {
            return figurList.remove(figur); 
        }
        
        return false;
    }

    /* (non-Javadoc)
     * @see Figur#equals(java.lang.Object)
     */
    @Override
    public boolean equals(Object param)
    {
        if (param == null)
        {
            return false; 
        }
        
        if (param == this)
        {
            return true; 
        }
        
        if (param.getClass() != getClass())
        {
            throw new ClassCastException(); 
        }
        
        boolean result = super.equals(param); 
        
        if (result == false)
        {
            return result; 
        }
        
        FigurContainer c = (FigurContainer)param; 
        
        /* Testen, ob es sich bei figurList um die gleiche handelt */
        if (figurList == c.figurList)
        {
            return true;
        }
        
        /* Kapazität vergleichen */
        if (capacity != c.capacity)
        {
            return false;
        }
        
        /* Anzal Figur-Elemente in der jeweiligen Liste vergleichen */
        if (figurList.size() != c.figurList.size())
        {
            return false; 
        }
        
        /* Einzelne Elemente in der jeweiligen Liste vergleichen */
        ArrayList<Figur> listThis = (ArrayList<Figur>)figurList;
        ArrayList<Figur> listCont = (ArrayList<Figur>)c.figurList;
        
        for (int i =0; i < listThis.size(); i++)
        {
            if (!listThis.get(i).equals(listCont.get(i)))
            {
                return false;
            }
        }
        return true;
    }
    
    /* (non-Javadoc)
     * @see Figur#compareTo(Figur)
     */
    @Override
    public int compareTo(Figur figur)
    {
        if (this == figur)
        {
            return 0; 
        }
        
        if (figur.getClass() != getClass())
        {
            throw new ClassCastException(); 
        }
        
        
        int result = super.compareTo(figur);
        
        if (result != 0)
        {
            return result;
        }
        
        FigurContainer c = (FigurContainer)figur;
        
        /* Testen, ob es sich bei beiden Objekte um die gleiche Liste handelt */
        if (figurList == c.figurList)
        {
            return 0; 
        }
        
        /* Kapazität vergleichen */
        if (capacity != c.capacity)
        {
            return capacity - c.capacity;
        }
        
        /* Die Anzahl Elemente vergleichen */
        if (figurList.size() != c.figurList.size())
        {
            return figurList.size() - c.figurList.size();
        }
        
        /* Einzelne Elemente in der jeweiligen Liste vergleichen */
        ArrayList<Figur> listThis = (ArrayList<Figur>)figurList;
        ArrayList<Figur> listCont = (ArrayList<Figur>)c.figurList;
        
        for (int i = 0; i < listThis.size(); i++)
        {
            int res = listThis.get(i).compareTo(listCont.get(i)); 
            
            if (res != 0)
            {
                return res;
            }
        }
        
        return 0; 
    }
}

Es würde mich interessieren, ob diese Implementierung (zumindest von der Idee her) brauchbar ist, oder befinde ich mich auf dem Holzweg bzw. habe ich da was wichtiges übersehen?

Vielen Dank.

 

[SlaterB]

brauchbar ist sie gewiss, denn du wirst das doch erfolgreich mit ein paar Test-Daten getestet haben,

wieso castest du aber die Collection auf ArrayList? wenn du derart ungeprüft davon ausgehst, dass es eine ArrayList ist,
dann wäre es nur konsequent, auch die Variable vom Typ ArrayList zu deklarieren


-------

public boolean remove(Figur figur)
    {
        if (figurList.contains(figur))
        {
            return figurList.remove(figur); 
        }
        
        return false;
    }

kürze:

public boolean remove(Figur figur)
    {
            return figurList.remove(figur); 
    }

 

[max5432]

Ja, ich habe es mit ein paar Daten schon getestet.

wieso castest du aber die Collection auf ArrayList? wenn du derart ungeprüft davon ausgehst, dass es eine ArrayList ist, ...

Nun, das ist es eben. Ich habe es allgemein gehalten, da die Möglichkeit offen gelassen werden muss, die Collection (aus welchem Grund auch immer) von Aussen zu setzen / ändern. Den Fall, dass mir jemand ein Set übergibt, müsste ich abfangen, da nur bein einer List-Instanz mit Index gearbeitet werden kann. Nun, wie macht man dies am besten?

Variante 1: Prüfen of eine List- oder eine Set-Instanz

if (figurList instanceof List)
{
    // mit Index arbeiten
}
else
{
   // ein Set übergeben: mit Iterator arbeiten
}

Variante 2: mit Iterator arbeiten (wie nachfolgend gezeigt)

public class FigurContainer extends Figur
{
    private Collection<Figur> figurs; 
    private int capacity; 
    
    public FigurContainer(int laenge, int breite, Collection<Figur> figurs, int capacity)
    {
        super(laenge, breite);
        this.figurs = figurs; 
        this.capacity = capacity;
    }

    public boolean add(Figur figur)
    {
        if (capacity > figurs.size())
        {
            return figurs.add(figur); 
        }
        
        return false;
    }
    
    public boolean remove(Figur figur)
    {
        if (figurs.contains(figur))
        {
            return figurs.remove(figur); 
        }
        
        return false;
    }

    /* (non-Javadoc)
     * @see Figur#equals(java.lang.Object)
     */
    @Override
    public boolean equals(Object param)
    {
        if (param == null)
        {
            return false; 
        }
        
        if (param == this)
        {
            return true; 
        }
        
        if (param.getClass() != getClass())
        {
            throw new ClassCastException(); 
        }
        
        boolean result = super.equals(param); 
        
        if (result == false)
        {
            return result; 
        }
        
        FigurContainer c = (FigurContainer)param; 
        
        /* Testen, ob es sich bei figurList um die gleiche handelt */
        if (figurs == c.figurs)
        {
            return true;
        }
        
        /* Kapazität vergleichen */
        if (capacity != c.capacity)
        {
            return false;
        }
        
        /* Anzal Figur-Elemente in der jeweiligen Liste vergleichen */
        if (figurs.size() != c.figurs.size())
        {
            return false; 
        }
        
        /* Einzelne Elemente in der jeweiligen Liste vergleichen */
        Iterator<Figur> iThis = figurs.iterator();
        Iterator<Figur> iCont = c.figurs.iterator();
        
        Figur fThis = null; 
        Figur fCont = null; 
        
        while(iThis.hasNext())
        {
            fThis = iThis.next();
            fCont = iCont.next();
            
            if (!fThis.equals(fCont))
            {
                return false;
            }
        }
        return true;
    }
    
    /* (non-Javadoc)
     * @see Figur#compareTo(Figur)
     */
    @Override
    public int compareTo(Figur figur)
    {
        if (this == figur)
        {
            return 0; 
        }
        
        if (figur.getClass() != getClass())
        {
            throw new ClassCastException(); 
        }
        
        
        int result = super.compareTo(figur);
        
        if (result != 0)
        {
            return result;
        }
        
        FigurContainer c = (FigurContainer)figur;
        
        /* Testen, ob es sich bei beiden Objekte um die gleiche Liste handelt */
        if (figurs == c.figurs)
        {
            return 0; 
        }
        
        /* Kapazität vergleichen */
        if (capacity != c.capacity)
        {
            return capacity - c.capacity;
        }
        
        /* Die Anzahl Elemente vergleichen */
        if (figurs.size() != c.figurs.size())
        {
            return figurs.size() - c.figurs.size();
        }
        
        /* Einzelne Elemente in der jeweiligen Liste vergleichen */
        Iterator<Figur> iThis = figurs.iterator();
        Iterator<Figur> iCont = c.figurs.iterator();
        
        Figur fThis = null; 
        Figur fCont = null; 
        
        while(iThis.hasNext())
        {
            fThis = iThis.next();
            fCont = iCont.next();
            
            if (!fThis.equals(fCont))
            {
                return fThis.compareTo(fCont);
            }
        }
        
        return 0; 
    }
}

Oder, was drittes?

 

[SlaterB]

wofür genau willst du eigentlich Listen sortieren?
dass das zufällig erste Element über die Position der Liste entscheidet scheint mir wenig sinnvoll,
da ist kaum eine Variante schlechter als die andere

bei Set.iterator() ist die Reihenfolge übrigens nicht garantiert, der nächste Durchlauf könnte andere Ergebnisse bringen

 

[max5432]

wofür genau willst du eigentlich Listen sortieren?
dass das zufällig erste Element über die Position der Liste entscheidet scheint mir wenig sinnvoll, da ist kaum eine Variante schlechter als die andere

Eine konkrete Anwendung dafür habe ich im Moment nicht, könnte aber bald zum Thema werden. Und das mit "sinnvoll", da hast du schon recht.

bei Set.iterator() ist die Reihenfolge übrigens nicht garantiert, der nächste Durchlauf könnte andere Ergebnisse bringen

Wird noch berücksichtigt.

Vielen Dank für die zahlreiche Hinweise.

 

[Landei]

Die Idee, dass du auf der Collection arbeitest, auf der auch "draußen" gearbeitet werden kann, ist gaaanz schlecht. Was denkst du, was passiert, wenn du gerade über die Collection iterierst, und anderswo (in einem anderen Thread) wird gerade daraus ein Element gelöscht?

Mach es doch so:

public class FigurContainer extends Figur
{
    private List<Figur> figurs; 
    private int capacity; 
    
    public FigurContainer(int laenge, int breite, Collection<Figur> figurs, int capacity)
    {
        super(laenge, breite);
        this.figurs = new ArrayList<Figur>(figurs); 
        this.capacity = capacity;
    }
   ... 
}

Damit erschlägst du alle drei Probleme: Der Nutzer kann eine Collection übergeben, du weißt, auf welcher Art Collection du arbeitest, und niemand kann dir von einem anderen Thread ins Handwerk pfuschen.

 

[SlaterB]

> und niemand kann dir von einem anderen Thread ins Handwerk pfuschen.

außer im Moment des Konstruktors

 

[Landei]

> und niemand kann dir von einem anderen Thread ins Handwerk pfuschen.

außer im Moment des Konstruktors

Soweit ich weiß laufen Konstruktoren und statische Initializer aus genau diesem Grund automatisch synchronisiert ab.

 

[SlaterB]

aber doch nicht synchronisiert auf alle Parameter und vielleicht noch deren enthaltenen Objekte usw.,
andererseits arbeitet new ArrayList(collection); ohne einen Iterator auf Collection sondern mit toArray(), da wirds schwer mit ConcurrentModificationException,
um die es doch geht oder?