compareTo-Sortierungsproblem

[MarsUM]

Hallo liebe Community!

ich habe ein kleines problem mit der sortier-fkt mittels compareTo.

1.
Ich möchte gerne zu meinem Audioplayer weitere Klassen hinzufügen, die meine Playlist nach Author, title, album und duration sortieren.

Also 4 weitere Klassen.

Ich möchte also die vom comparator vorgeschriebene Methode "public int compare (audiofile af1, audiofile af2)" so ersetzen, dass der rückgabewert dem ergebnis des Vergleiches der attribute author der beiden übergebenen audiofiles mittels der compareTo-Methode entspricht.
Das selbe wäre ja dann auch bei title analog dazu zu machen...

habe mir einen solchen ansatz mal überlegt:

import java.util.Comparator;

public abstract class AuthorComparator implements Comparable<AuthorComparator>{


	  public int compareTo(AudioFile af1)
	     {
	    		 return af1.getAuthor().compareTo(author); 		 
	     }

}

Allerdings meckert meine IDE beim letzten "author". Was stimmt da nicht?


2.
Beim Sortieren nach Album und Duration wird das ganze etwas komplizierter...

die für den Vergleich benötigten Attribute sind nicht in der (abstrakten Eltern-)Klasse AudioFile definiert.
Daher kann nicht für alle zu vergleichenden Elemente auf die benötigten Attribut zugegriffen werden.
Genauer ist das Attribut duration in meiner vorher angelegten Klasse SampledFile, und das Attribut album in der Klasse TaggedFile festgelegt.
Aus diesem Grund ist vor dem eigentlichen Vergleich eine Überprüfung nötig, ob es sich bei den beiden zu vergleichenden Objekten um Objekte der jeweils benötigten (Kind-)Klasse handelt.

Sollte das erste übergebene (oder beide) Objekt nicht vom benötigten Typ sein, würde ich gerne -1 "returnen"
Sollte das zweite Objekt nicht vom benötigten Typ seim -> return 1.
Sind beide Objekte vom benötigten Typ, gibt es wieder den Vergleich...

Da in der Elternklasse das benötigte Attribut jedoch nicht bekannt ist, ist ein Cast, also ein
explizites Umwandeln in den entsprechenden Typ nötig. So dachte ich mir, dass ich etwa mit "((TaggedFile)af1).album" ein AudioFileObjekt af1 als TaggedFile anspreche und auf das Attribut album zugreife.

Jetz würde ich gern die Klasse AlbumComparator für die Attribute album analog zu
den bereits implementierten Comparator-Klassen implementieren. Wie mache ich zusätzlich da noch die überprunf via instanceof rein??

Bei der Klasse "DurationComparator" handelt es sich ja nicht um String-Attribute; wie kann ich hier die Differenz af1.duration–af2.duration zurückgeben?
Ich schätze mal dass ich dabei wieder den Cast des Ergebnisses nach int machen muss.


Kann mir da vielleicht jemand weiterhelfen?
Habe vorallem beim zweiten Teil eher nur eine vage Vermutung, wie ich da hinkriege...

Vielen Dank,
Schönen Abend.

 

[Marco13]

Zu 1: Es bringt nichts, einen Comparator Comparable sein zu lassen. Implementiere nicht Comparable, sondern Comparator... DER kriegt in der compare-Methode auch ZWEI Objekte übergeben

Zu 2: Mit
[c]if (af1 instanceof TaggedDingens)[/c]
kann man prüfen, ob ein Objekt von der Klasse TaggedDingens ist, und das Objekt dann ggf. mit
[c]TaggedDingens t1 = (TaggedDingens)af1;[/c]
casten.


Bei der Klasse "DurationComparator" handelt es sich ja nicht um String-Attribute; wie kann ich hier die Differenz af1.duration–af2.duration zurückgeben?
Ich schätze mal dass ich dabei wieder den Cast des Ergebnisses nach int machen muss.

Nicht so gut, da kann Mist rauskommen. Bei durations d0 und d1 lieber sowas machen wie

if (d0 > d1) return 1;
if (d0 < d1) return -1;
return 0;

 

[MarsUM]

Bin ich damit also auf dem richtigen Weg:

public abstract class AlbumComparator implements Comparator<AlbumComparator>{


	public int compareTo(TaggedFile af1, TaggedFile af2)
	{
	    if(!(af2 instanceof TaggedFile))
	        return 1;
	    if(!(af1 instanceof TaggedFile))
	        return -1;
	    if((af1 instanceof TaggedFile) && (af1 instanceof TaggedFile))
	        return af1.getAlbum().compareTo(((TaggedFile)af1).getAlbum());
	    else
	        return 0;
	}

}

Btw: Gibt es eine Möglichkeit die Klasse nicht abstract zu machen? Wenn ja, wie?
Wenn ich das abstract rauslösche, meckert meine IDE und unterringelt es.

 

[Marco13]

Boah, ist schon spät - im Moment kapier' ich nicht mal, warum er das überhaupt compiliert... eigentlich sollte er das nicht... Aber spätestens, wenn man den verwenden wollte, würde es vermutlich krachen... eigentlich sollte es eher

public abstract class AlbumComparator implements Comparator<AudioFile >{

sein. Der Vergleich an sich stimmt wohl auch nicht nicht, aber ... kannst ja nochmal schauen...

 

[MarsUM]

Sodala,
ich glaub ich bin auf einem ganz gutem Weg, denn eine TestUnit beschwert sie nur noch am Sortieren nach Interpret.

Zum Verständnis:

Ich sortiere nach Interpret mit folgendem Code:

import java.util.Comparator;

public class AuthorComparator implements Comparator<AudioFile>{


	  public int compare(AudioFile af1, AudioFile af2)
	     {
	    		 return af1.getAuthor().compareTo(af2.getAuthor()); 		 
	     }

}

(Bei der Titel-Sortierung ist es genau der selbe Code; entsprechend "title" statt "author".)

Sortiert werden die Lieder mithilfe eines Collections.sort-Aufrufes in einer anderen Klasse:

    public void sort(SortCriterion order)
    {
        Collections.sort(this, new AuthorComparator()); 
        Collections.sort(this, new TitleComparator());
        Collections.sort(this, new AlbumComparator());
        Collections.sort(this, new DurationComparator());
    }

Die SortCriterion ist ziemlich simpel gehalten und enthält nur die Aufzählung der Sortiermöglichkeiten:

public enum SortCriterion {AUTHOR, TITLE, ALBUM, DURATION}

Soweit so gut....

Lasse ich eine entsprechende Testunit drüber laufen, murkst mir das programm nur bei der Sortierung nach Interpret!
Wie kann das sein? Ich meine, wieso funktioniert die Titel-Sortierung aber nicht die der Interpreten?? Ist doch genau das selbe in grün!

Zum Verständnis vielleicht nochmal der Code der TestUnit:

    public void testSorting() {
        PlayList pl1 = new PlayList();
        try {
            pl1.add(new TaggedFile("Rock 812.mp3"));
            pl1.add(new WavFile("wellenmeister - tranquility.wav"));
            pl1.add(new TaggedFile("wellenmeister_awakening.ogg"));
        } catch (NotPlayableException e) {
            fail("Audiodatei konnte nicht erzeugt werden");
        }
        pl1.sort(SortCriterion.ALBUM);
        assertEquals(
                "Sortierung nach Album nicht richtig",
                "[wellenmeister - tranquility - 02:21, "
                + "Eisbach - Rock 812 - The Sea, the Sky - 05:31, "
                + "Wellenmeister - TANOM Part I: Awakening - TheAbsoluteNecessityOfMeaning - 05:55]",
                pl1.toString());
        pl1.sort(SortCriterion.AUTHOR);
        assertEquals(
                "Sortierung nach Interpret nicht richtig",
                "[Eisbach - Rock 812 - The Sea, the Sky - 05:31, "
                + "Wellenmeister - TANOM Part I: Awakening - TheAbsoluteNecessityOfMeaning - 05:55, "
                + "wellenmeister - tranquility - 02:21]", pl1
                .toString());
        pl1.sort(SortCriterion.DURATION);
        assertEquals(
                "Sortierung nach Dauer nicht richtig",
                "[wellenmeister - tranquility - 02:21, "
                + "Eisbach - Rock 812 - The Sea, the Sky - 05:31, "
                + "Wellenmeister - TANOM Part I: Awakening - TheAbsoluteNecessityOfMeaning - 05:55]",
                pl1.toString());
        pl1.sort(SortCriterion.TITLE);
        assertEquals(
                "Sortierung nach Titel nicht richtig",
                "[Eisbach - Rock 812 - The Sea, the Sky - 05:31, "
                + "Wellenmeister - TANOM Part I: Awakening - TheAbsoluteNecessityOfMeaning - 05:55, "
                + "wellenmeister - tranquility - 02:21]", pl1
                .toString());
    }

Sieht einer den Fehler, oder bin ich einfach zu blind :autsch: und es ist zu spät in der nacht?!? :rtfm:

 

[Landei]

Wenn du nur nach Autor sortierst, sortierst du nur nach Autor, d.h. es ist undefiniert, ob Beatles - Lucy in the Sky with Diamonds vor oder nach Beatles - Yellow Submarine kommt. Mehrere sort-Aufrufe hintereinander sind sinnlos, die Sortierungen sind nicht irgendwie "kumulativ", sondern die letzte überschreibt alle anderen.

 

[Marco13]

Da kann man sich aber was basteln...

import java.util.*;


public class ChainedComparatorTest
{
    public static void main(String[] args)
    {
        List<Element> list = new ArrayList<Element>();

        list.add(new Element("AA", "AA", "AA"));
        list.add(new Element("AA", "AA", "BB"));
        list.add(new Element("AA", "BB", "AA"));
        list.add(new Element("BB", "AA", "BB"));
        list.add(new Element("BB", "AA", "CC"));
        list.add(new Element("CC", "AA", "BB"));

        Collections.shuffle(list);

        Comparator<Element> c0 = new Comparator<Element>()
        {
            public int compare(Element o1, Element o2)
            {
                return o1.getFirst().compareTo(o2.getFirst());
            }
        };
        Comparator<Element> c1 = new Comparator<Element>()
        {
            public int compare(Element o1, Element o2)
            {
                return o1.getSecond().compareTo(o2.getSecond());
            }
        };
        Comparator<Element> c2 = new Comparator<Element>()
        {
            public int compare(Element o1, Element o2)
            {
                return o1.getThird().compareTo(o2.getThird());
            }
        };

        System.out.println("Before");
        for (Element e : list)
        {
            System.out.println(e);
        }


        Comparator<Element> c = c0;
        c = new CompoundComparator<Element>(c, c1);
        c = new CompoundComparator<Element>(c, c2);

        Collections.sort(list, c);

        System.out.println("After");
        for (Element e : list)
        {
            System.out.println(e);
        }

    }
}



class CompoundComparator<T> implements Comparator<T>
{
    Comparator<T> delegate0;
    Comparator<T> delegate1;

    public static <T> Comparator<T> create(Comparator<T> ... delegates)
    {
        if (delegates == null || delegates.length < 2)
        {
            throw new IllegalArgumentException("Invalid delegates");
        }
        Comparator<T> current = delegates[0];
        for (int i=1; i<delegates.length; i++)
        {
            current = new CompoundComparator<T>(current, delegates[i]);
        }
        return current;
    }

    public CompoundComparator(Comparator<T> delegate0, Comparator<T> delegate1)
    {
        this.delegate0 = delegate0;
        this.delegate1 = delegate1;
    }

    @Override
    public int compare(T o1, T o2)
    {
        int result = delegate0.compare(o1, o2);
        if (result != 0)
        {
            return result;
        }
        return delegate1.compare(o1, o2);
    }



}



class Element
{
    private String first;
    private String second;
    private String third;

    public Element(String first, String second, String third)
    {
        super();
        this.first = first;
        this.second = second;
        this.third = third;
    }

    public String getFirst()
    {
        return first;
    }
    public String getSecond()
    {
        return second;
    }
    public String getThird()
    {
        return third;
    }

    public String toString()
    {
        return first + " " +second + " " +third;
    }

}

(In Scala wär' das vermutlich ein Dreizeiler )

 

[MarsUM]

Wenn du nur nach Autor sortierst, sortierst du nur nach Autor, d.h. es ist undefiniert, ob Beatles - Lucy in the Sky with Diamonds vor oder nach Beatles - Yellow Submarine kommt. Mehrere sort-Aufrufe hintereinander sind sinnlos, die Sortierungen sind nicht irgendwie "kumulativ", sondern die letzte überschreibt alle anderen.

also du meinst, dass mein Code stimmt, aber dass es auf die art und weise nicht zu einem richtigem ergebnis kommen wird?!
Gibt es nicht noch einen anderen weg, ohne die array-liste neu anzulegen? immerhin lege ich die ja nur indirekt selber an, da die playlist in der testunit ja selbst erzeugt wird.
Mein code soll diese playlist aufgreifen, und dann mit collections.sort neu sortieren.
:rtfm:

 

[Marco13]

Im oben geposteten Codeschnipsel ist der "CompoundComparator", mit dem man mehrere Comparatoren (also mehrere Sortierkriterien) zu einem Kombinieren kann. Ist vielleicht ein bißchen Overkill, das könnte man auch mit einer anonymen inneren Klasse machen, aber ... naja...

 

[faetzminator]

Wieso nicht einfach folgendes:

[...]
int cmp = this.getAuthor().compareTo(other.getAuthor());
if (cmp != 0) {
    return cmp;
}
cmp = this.getAlbum().compareTo(other.getAlbum());
if (cmp != 0) {
    return cmp;
}
// ...
return this.getFoo().compareTo(other.getFoo());

 

[Marco13]

.... und wenn man n Sortierkriterien hat, und jede beliebige Priorisierung anbieten will, baut man sich n*(n-1)/2 Compartoren

 

[faetzminator]

Man kann ein Enum oä anbieten, welches alle Felder abdeckt. Alle Values können intern in einer Map oä abgebildet werden und der Zugriff (wenn gehashed) ist sozusagen gleich schnell (O(1)). Mit einer List der gewünschten Sortierung kann so alles mit einem Comparator abgebildet werden.

 

[MarsUM]

Man kann ein Enum oä anbieten, welches alle Felder abdeckt. Alle Values können intern in einer Map oä abgebildet werden und der Zugriff (wenn gehashed) ist sozusagen gleich schnell (O(1)). Mit einer List der gewünschten Sortierung kann so alles mit einem Comparator abgebildet werden.

Sorry, bin noch ziemlicher Java-Anfänger, versteh beim meisten was ihr redet nur bahnhof...:bahnhof:

Wenn du das hier mit der Map meinst:

[JAVA=25] Map map = TagReader.readTags(super.getPathname());
if(map.get("album") != null)
setAlbum((String)map.get("album"));
else
setAlbum("");
if(map.get("title") != null)
super.setTitle((String)map.get("title"));
else
super.setTitle("");
if(map.get("author") != null)
super.setAuthor((String)map.get("author"));
else
super.setAuthor("");
Long durationtime = (Long)map.get("duration");
if(durationtime != null)
super.setDuration(durationtime.longValue());[/code]

das ist ein ausschnitt aus der TaggedFile.


Dein Code vom Post davor, murkst bei mir in der IDE noch mehr rum...

Müsste es dann so heißen:

public int compareTo(AudioFile af1, AudioFile af2){
int cmp = af1.getAuthor().compareTo(af2.getAuthor());
if (cmp != 0) {
    return cmp;
}
}

?

Oder sonst zu meinem eigentlichen Code einen verbesserungsvorschlag?

 

[MarsUM]

Soweit bin ich jetz schon:

Der Fehler muss hier liegen:

[JAVA=122] public void sort(SortCriterion order)
{
Collections.sort(this, new AuthorComparator());
Collections.sort(this, new TitleComparator());
Collections.sort(this, new AlbumComparator());
Collections.sort(this, new DurationComparator());
}[/code]

Ich sortiere hier immer zunächst nach Author, dann
nach Titel, dann nach Album, dann nach Dauer. Ich will aber immer nur nach
einem von diesen vieren sortieren, nämlich den durch den Parameter "order"
festegelegten.

Wie, bzw Wo kriegt ich order mit rein?

 

[SlaterB]

Comparator c = null;
switch (order) {
c setzen
}
Collections.sort(this, c);


Switch statement with enum : enum methodsData TypeJava Tutorial

 

[MarsUM]

Switch statement with enum : enum methodsData TypeJava Tutorial

Danke, hat mir schon einmal weiter geholfen. Schaut jetzt bei mir so aus:

    public void sort(SortCriterion order)
    {
    	Choice order = Choice.AUTHOR;
    	
    	switch (order){
    	case AUTHOR:
    	        Collections.sort(this, new AuthorComparator());
    	        break;
    	case TITLE:
    			Collections.sort(this, new TitleComparator());
    			break;
    	case ALBUM:
    			Collections.sort(this, new AlbumComparator());
    			break;
    	case DURATION:
    	        Collections.sort(this, new DurationComparator());
    	        break;
    	}
    }

Allerdings mag er das 'Choice order = Choice.AUTHOR' noch nicht so ganz.

Liegt wahrscheinlich daran, dass die enum-Aufzählung in einer anderen Klasse liegt:

public class SortCriterion 
{enum Choice {AUTHOR, TITLE, ALBUM, DURATION}
}

ideas?? :rtfm:

 

[SlaterB]

Zeile 3 komplett weg, Rest läuft

bzw. SortCriterum muss auch wieder
public enum SortCriterion {AUTHOR, TITLE, ALBUM, DURATION}
lauten,

der Link war nur ein Beispiel, du musst nicht auf einmal 'Choice' bei dir reinkopieren