Probleme im Umgang mit PriorityQueue

[xImJugo]

Wieso ständig? Du brauchst das nur ein einer einzelnen Stelle, da wo die Liste ausgegeben wird.
Für alle anderen Methoden ist die Reihenfolge egal.

Ich hab trotzdem keine Ahnung wie ich dann durch die Elemente iterieren soll oder sollte ich das dann mit dem Iterator wie ganz am Anfang machen ? Stand jetzt habe ich zum Beispiel für add das :

public void addSong(Song song) {

        ArrayList<Song> newQueue = new ArrayList<Song>();
        if (newQueue.size() == 0) {
            newQueue.add(song);
        } else {
            for (int i = 0; i < newQueue.size(); i++) {
            // equals methode ist überschrieben
                if (newQueue.get(i).equals(song) || newQueue.get(i).getId() != song.getId()) {
                    newQueue.add(song);
                }
            }
        }

        for (int i = 0; i < newQueue.size(); i++) {
            queue.add(newQueue.get(i));
        }

    }

Ist das so nicht gut ?

Play ist dafür doch völlig egal?

die Restzeit brauchst du nicht im Song speichern - es hat ja nicht jeder einzelne Song eine Restzeit, sondern nur der aktuelle. Kann je nach Umsetzung einfacher sein.

Also ist Resttime dann ein Attribut in der PQ Klasse und nicht der Song Klasse ? Wie würdest du das dann umsetzen

Wenn mehr Zeit gespielt werden soll, als in aktuellem Lied übrig (kannst du prüfen), dann musst du berechnen, wie viel Zeit nach dem aktuellen Lied noch gespielt werden musst, das nächste Lied aus der Liste holen, und die Zeit davon abziehen.
Einfach mal auf Papier durchspielen.




Na gut, Play ist nicht am einfachsten

Im Kopf ist mir das ja schon klar, wie das auszusehen hat, aber hab schon einige Sachen versucht. Aber ich schätze mal ich sollte dafür Hilfsmethoden verwenden und nicht alle Überprüfungen in eine methode klatschen ?

 

[lennero]

Ich hab trotzdem keine Ahnung wie ich dann durch die Elemente iterieren soll oder sollte ich das dann mit dem Iterator wie ganz am Anfang machen ? Stand jetzt habe ich zum Beispiel für add das :

public void addSong(Song song) {

        ArrayList<Song> newQueue = new ArrayList<Song>();
        if (newQueue.size() == 0) {
            newQueue.add(song);
        } else {
            for (int i = 0; i < newQueue.size(); i++) {
            // equals methode ist überschrieben
                if (newQueue.get(i).equals(song) || newQueue.get(i).getId() != song.getId()) {
                    newQueue.add(song);
                }
            }
        }

        for (int i = 0; i < newQueue.size(); i++) {
            queue.add(newQueue.get(i));
        }

    }

Ist das so nicht gut ?

Ne, du hast da toten code drin.

 

[xImJugo]

Ne, du hast da toten code drin.

Wo denn ? Hätte ich solchen hätte eclipse es mir angezeigt, aber alles funktioniert da. Aber es ist halt hässlich af

 

[httpdigest]

Eclipse implementiert keinerlei Dataflow Analyse, um sowas zu erkennen:

List list = new ArrayList();
if (list.isEmpty()) { // bzw. wie bei dir: list.size() == 0
  // always true
} else {
  // always false
}

Wenn du viel Hilfe in dieser Hinsicht willst, nimm IntelliJ IDEA. Der schlägt dir so einige Verbesserungen/Vereinfachungen zu deinem Code vor.

Dein obiger Code durch automatische Vereinfachungen durch IntelliJ IDEA angepasst:

public void addSong(Song song) {
    ArrayList<Song> newQueue = new ArrayList<>();
    newQueue.add(song);
    queue.addAll(newQueue);
}

Kann auch noch zu:

public void addSong(Song song) {
    queue.add(song);
}

vereinfacht werden.

 

[mrBrown]

Ist das so nicht gut ?

Abgesehen von den schon gegebenen Hinweisen: du brauchst dort keine neue Liste/Queue, die vorhandene zu nutzen reicht völlig aus.

Also ist Resttime dann ein Attribut in der PQ Klasse und nicht der Song Klasse ? Wie würdest du das dann umsetzen

Als Attribute in der Playlist, oder besser in einer extra „Player“-Klasse, wenn es eine gäbe.

Im Kopf ist mir das ja schon klar, wie das auszusehen hat, aber hab schon einige Sachen versucht. Aber ich schätze mal ich sollte dafür Hilfsmethoden verwenden und nicht alle Überprüfungen in eine methode klatschen ?

Sollten ist ein hartes Wort, sinnvoll ist das schon, aber kein Muss.

Erklär es doch mal in Worten.

 

[kneitzel]

Das mit der zweiten Queue geht wohl etwas auf meine Beiträge zurück - das bezieht sich nur auf das iterieren über eine Collection mit löschen eines Elements. Bei dem Hinzufügen brauchst Du sowas überhaupt nicht, das sollte die Struktur schaffen - und das in der Regel ohne durch die Werte zu iterieren.

Wenn das missverständlich war, dann tut es mir leid.

 

[xImJugo]

Erstmal sorry an alle, dass ich jetzt erst wieder antworte und gestern nichts kam, aber gestern war einfach ein sehr stressiger Tag -._

Wenn mehr Zeit gespielt werden soll, als in aktuellem Lied übrig (kannst du prüfen), dann musst du berechnen, wie viel Zeit nach dem aktuellen Lied noch gespielt werden musst, das nächste Lied aus der Liste holen, und die Zeit davon abziehen.
Einfach mal auf Papier durchspielen.

Leider komme ich echt nicht drauf, was für eine einfache Lösung du hier siehst :

 public void play(int length) {
        this.playedSong = queue.peek();   // oberster Song aus der PQ wird genommen

        if (playedSong.getLength() < length) { // wenn die gegebene Länge größer ist als die Länge des Songs, dann muss der oberster aufjeden Fall runter genommen werden und die restzeit bestimmt werden 
            queue.poll();
            int restTime = length - playedSong.getLength();
        }

    }

So in etwa sollte es gehen ? Aber mir fehlt das automatisierte, also eine Schleife oder etwas in der Art, damit es solange das Playen simuliert, bis die zeit vorbei ist. Kannst du mir da doch aushelfen

 

[xImJugo]

Das mit der zweiten Queue geht wohl etwas auf meine Beiträge zurück - das bezieht sich nur auf das iterieren über eine Collection mit löschen eines Elements. Bei dem Hinzufügen brauchst Du sowas überhaupt nicht, das sollte die Struktur schaffen - und das in der Regel ohne durch die Werte zu iterieren.

Wenn das missverständlich war, dann tut es mir leid.

Da kannst du nichts für, also keine Sorge. Ich bin nur überfordert mit der PQ, sodass ich mich bewusst auf das Kopieren zur ArrayList entschieden habe, da ich mit der umgehen kann. Brauch ich jetzt doch nur einen einfachen Iterator, der das iterieren übernimmt ?

 

[xImJugo]

Abgesehen von den schon gegebenen Hinweisen: du brauchst dort keine neue Liste/Queue, die vorhandene zu nutzen reicht völlig aus.



Als Attribute in der Playlist, oder besser in einer extra „Player“-Klasse, wenn es eine gäbe.

Was würde denn die Player klasse beinhalten und was wäre ihr Sinn ? Weil ich seh den Unterschied von meiner PQ Klasse und dem Player nicht

Sollten ist ein hartes Wort, sinnvoll ist das schon, aber kein Muss.

Erklär es doch mal in Worten.

Also in Worten erklärt : Die PQ Klasse betrachtet den obersten Song und vergleicht diese Länge mit der angegebene Länge. Sind die beiden gleich lang (einfachster Fall), dann wird der aktuelle Song gepollt und in die history gepackt, fertig. Wenn die Songlänge kleiner ist als die geforderte Länge, dann wird der aktuelle Song auch gepollt und history hinzugefügt, aber es muss dann automatisch der nächste Song (Also der neue höchste) genommen werden und wieder mit der RestLänge verglichen werden usw, bis eben die geforderte Länge erfüllt bzw abgelaufen ist. Erst dann gilt der Play Befehl als erfolgreich.
Das alles klingt schon nach einer Schleife und paar If Abfragen, aber ich bin zu dumm für die Umsetzung


Edit: Habs jetzt so versucht mit dem Betrag aber passt das so mehr oder weniger ?

public void play(int length) {

        while (length != 0) {
            this.playedSong = queue.peek();

            if (playedSong.getLength() < length) {
                queue.poll();
                int restTime = length - playedSong.getLength();
                length = Math.abs(restTime);
            }
        }

    }

Edit 2.0 ich bin lost und es kann so auch nicht sein

 

[xImJugo]

Eclipse implementiert keinerlei Dataflow Analyse, um sowas zu erkennen:

List list = new ArrayList();
if (list.isEmpty()) { // bzw. wie bei dir: list.size() == 0
  // always true
} else {
  // always false
}

Wenn du viel Hilfe in dieser Hinsicht willst, nimm IntelliJ IDEA. Der schlägt dir so einige Verbesserungen/Vereinfachungen zu deinem Code vor.

Dein obiger Code durch automatische Vereinfachungen durch IntelliJ IDEA angepasst:

public void addSong(Song song) {
    ArrayList<Song> newQueue = new ArrayList<>();
    newQueue.add(song);
    queue.addAll(newQueue);
}

Kann auch noch zu:

public void addSong(Song song) {
    queue.add(song);
}

vereinfacht werden.

Danke auch dir für die Tipps und Kommentare, aber bevor ich zu add gehe, will ich erstmal diese play methode hinbekommen. Aber ich werd auf dich zurückkommen

 

[xImJugo]

Also Stand jetzt bin bei dem Konstrukt :

public void play(int length) {

        while (length > 0) {
            this.playedSong = queue.peek();

            if (playedSong.getLength() < length) {
                historyOfQueue.add(queue.poll());
                length = length - playedSong.getLength();
            }
        }

    }

Aber noch was allgemeines, woher weiß ich wann void sinnvoll ist als Rückgabewert (also nichts) oder ob ich doch etwas zurückgeben sollte ? Bei play oder add oder skip biete sich zum Bsp boolean an oder nicht ? Bei remove könnte man einen Song zurückgeben oder boolean, aber ich benutze jetzt eigentlich fast immer void (bei list Befehl bin ich mir sicher, dasss es void ist). Aber zum Beispiel habe ich bei skip void, obwohl boolean auch gehen würde

 

[xImJugo]

Also es tut mir leid für den Spam, aber ich wollte nicht abgehoben sein, deswegen musste ich jedem ein mal antworten und auch hier nochmal danke für die bisherige Hilfe. Ich bin jetzt auf dem Stand, dass alle Methoden bis auf add wahrscheinlich klappen. Wie schön ich die anderen Methoden (auch play) umgesetzt habe, könnt ihr beurteilen. Also falls ihr es sehen wollt, kann ich den Code sobald wer wieder am Start ist, reinposten. aber meiner Ansicht nach klappt alles, aber die add Methode schaff ich definitiv nicht ohne eure Hilfe und wäre demnach sehr glücklich, wenn jemand mir da aushilft

 

[xImJugo]

Kann oder will mir niemand beim letzten oder den letzten Schritt helfen ? ich hab massive Probleme bei der add Methode, da hier einfach gar nichts klappt

 

[mrBrown]

Wie sieht denn dein Ansatz für die add-Methode aus?

 

[xImJugo]

Also erstmal zur Add Methode : Es soll ja ein gegebenes Lied der Liste hinzugefügt werden. Dabei müssen folgende Bedingungen erfüllt sein : 1. Gleiche Lieder ( also wo alle Attribute übereinstimmen dürfen hinzugefügt werden)
2. Wenn Lieder unterschiedliche Kennungen haben dürfen sie auch ohne Probleme hinzugefügt werden
3. Wenn Lieder gleiche Kennungen haben, müssen auch die restlichen Attribute übereinstimmen
4. Wenn ein Lied gerade läuft bzw simuliert wird, wird ein Lied mit höherer Priorität hinter dem angereiht (also laufende Lieder werden nicht unterbrochen)

Wie ich 4. überprüfe oder einbaue, habe ich keine Ahnung, aber zu den anderen 3 Punkten hatte ich 2 Ansätze :
1. Durch die PQ iterieren, wobei das immernoch nicht gescheit klappt, und prüfen, ob die Bedingungen 1-3 erfüllt sind und wenn ja hinzufügen

oder

2. Ich füge alle Lieder hinzu und lass dann einen Iterator drüber laufen und Lieder, die die Bedingungen brechen, löschen :

public void addSong(Song song) {
        queue.add(song);
        Iterator<Song> iterator = queue.iterator();

        while (iterator.hasNext()) {
            if (iterator.next().getId() == song.getId() && (!iterator.next().getArtist().equals(song.getArtist())
                    || !iterator.next().getTitle().equals(song.getTitle()))) {
                iterator.remove();
            }
        }
    }
    // liefert Fehler bzw Exception

Aber beides klappt nicht /:

 

[mrBrown]

1-3 kann man doch zusammenfassen mit "Wenn das Lied nicht enthalten ist, wird es hinzugefügt", und den "nicht enthalten"-Teil kann man dann doch wunderschön auslagern.

4 ist etwas tricky - du musst dir noch mal bewusst machen, wie die Lieder in der PriorityQueue angeordnet sind.

 

[xImJugo]

1-3 kann man doch zusammenfassen mit "Wenn das Lied nicht enthalten ist, wird es hinzugefügt", und den "nicht enthalten"-Teil kann man dann doch wunderschön auslagern.

Kann man doch eben nicht, da man gleiche Lieder mehrfach hinzufügen darf oder denk ich falsch ? Aber ich hab deinen Denkanstoß verstanden, da ich jetzt erst gecheckt habe, dass ich die .contains Methode sinnvoll benutzen kann.
Ein Teil des Codes müsste auf jeden Fall so aussehen :

public void addSong(Song song) {
         
        // Wenn ein Song nicht in der Liste ist, wird dieser hinzugefügt
        if (!queue.contains(song) ) {
            queue.add(song);
        }
        // Deckt aber nicht den Fall ab, wenn es sich um 2 gleiche handelt
    }

Es ist schon sinnvoll, dass meine equals Methode so überschrieben ist, dass sie ein Lied auf Id, Titel, Sänger und Länge prüft oder ? oder sollte es nur auf id prüfen ?

wenn ich als Beispiel das habe :

musicplayer.addSong(new Song(2, "Alice", "Spam", 200, 2));
musicplayer.addSong(new Song(2, "Bob", "Spam", 200, 2));

Dann darf der 2. nicht hinzugefügt werden, aber bei mir ist das trotzdem möglich. Wie kann ich das umgehen

4 ist etwas tricky - du musst dir noch mal bewusst machen, wie die Lieder in der PriorityQueue angeordnet sind.

Leider versteh ich diesen Hinweis nicht. Hatte überlegt mit der Resttime ( Attribut von PQ Klasse) zu spielen oder auch in eine Hilfsmethode in der Song Klasse zu implementieren , die überprüft ob es gespielt wird, aber das ist noch eine echte Baustelle

 

[mrBrown]

Kann man doch eben nicht, da man gleiche Lieder mehrfach hinzufügen darf oder denk ich falsch ? Aber ich hab deinen Denkanstoß verstanden, da ich jetzt erst gecheckt habe, dass ich die .contains Methode sinnvoll benutzen kann.

Nein, contains kann man dafür nicht nutzen, wie du ja selbst sagst deckt das einen Fall nicht ab.

1-3 kannst du auf zwei Dinge aufteilen:
- den Test, ob ein Lied hinzugefügt werden darf
- das eigentliche hinzufügen

Als Test nutzt du bisher !queue.contains(song), das klappt wie gesagt nicht, sondern du muss das selbst implementieren. Das machst du aber sinnvoller Weise nicht in der add-Methode, sondern schreibst eine eigene dafür, etwa private boolean contains(Song song) {...}.

Leider versteh ich diesen Hinweis nicht.

Erklär doch einfach mal, wie die Lieder in der PriorityQueue angeordnet sind.

 

[xImJugo]

Nein, contains kann man dafür nicht nutzen, wie du ja selbst sagst deckt das einen Fall nicht ab.

1-3 kannst du auf zwei Dinge aufteilen:
- den Test, ob ein Lied hinzugefügt werden darf
- das eigentliche hinzufügen

Als Test nutzt du bisher !queue.contains(song), das klappt wie gesagt nicht, sondern du muss das selbst implementieren. Das machst du aber sinnvoller Weise nicht in der add-Methode, sondern schreibst eine eigene dafür, etwa private boolean contains(Song song) {...}.

Diese Hilfsmethode kommt dann in die PQ klasse oder Song Klasse ?

Erklär doch einfach mal, wie die Lieder in der PriorityQueue angeordnet sind.

Die Priority queue wird entsprechend der Priorität von 0 bis 5 angeordnet, wobei 0 das höchste ist und 5 die kleinste, aber inwiefern hilft das

 

[mrBrown]

Diese Hilfsmethode kommt dann in die PQ klasse oder Song Klasse ?

In deine Playlist-Klasse. Ob etwas in der Playlist enthalten ist, ist doch Verantwortlichkeit der PlayList

Die Priority queue wird entsprechend der Priorität von 0 bis 5 angeordnet, wobei 0 das höchste ist und 5 die kleinste, aber inwiefern hilft das

Und wenn jetzt ein Song Priorität 7 hat?

 

[xImJugo]

In deine Playlist-Klasse. Ob etwas in der Playlist enthalten ist, ist doch Verantwortlichkeit der PlayList

Ja war ne dumme Frage. In der Song Klasse macht es keinen Sinn, aber ich bin gerade wirklich genervt, weil ich zu dumm für diese Methode bin. Wieso macht folgendes nicht das, was es tun soll :

public boolean isValid(Song song) {
        Iterator<Song> iterator = queue.iterator();
        while (iterator.hasNext()) {
            // Das prüft doch die ID von den Songs in der PQ und dem neuen
            // Wenn die Id mit keinem Song übereinstimmt, dann darf der Song doch auch hinzugefügt werden
            if (iterator.next().getId() != song.getId()) {
                return true;
            }
            // hier prüft es ob ein gleicher Song bereits vorliegt
            // wenn ja dann ist auch gut
            if (iterator.next().equals(song)) {
                return true;
            }
        }
        Wenn nichts davon zutrifft dann false
        return false;
    }

Wenn ich jetzt als Beispiel wieder :
Song s1 = new Song(2, "Bob", "Spam", 200, 1);
musicplayer.addSong(new Song(2, "Alice", "Spam", 200, 2));
Jetzt sollte die Methode false liefern, da die Songs die gleiche Kennung haben aber unterschiedliche Attribute , aber sie liefert mir true und ich check nicht wieso

Und wenn jetzt ein Song Priorität 7 hat?

7 geht gar nicht. Hab einen setter dafür, der nur werte erlaubt, die auch erlaubt sind

 

[mrBrown]

Warum nimmst du überhaupt den Iterator? Kannst auch einfach eine ganz normale Schleife nutzen.
Das Problem ist der doppelte Aufruf von iterator.next, das darf nach jedem hasNext-Aufruf nur ein einziges Mal aufgerufen werden.

7 geht gar nicht. Hab einen setter dafür, der nur werte erlaubt, die auch erlaubt sind

Achso, also Priorität liegt immer zwischen 0 und 5, wobei 5 die höchste ist?

 

[xImJugo]

Warum nimmst du überhaupt den Iterator? Kannst auch einfach eine ganz normale Schleife nutzen.
Das Problem ist der doppelte Aufruf von iterator.next, das darf nach jedem hasNext-Aufruf nur ein einziges Mal aufgerufen werden.

Also war eine schwere Geburt, aber ich hab jetzt was, was die ersten 3 Punkte zu erfüllen scheint. Wollte es so kurz und schön wie möglich haben, aber es wurde einfach hässlich und lang in der Hilfsmethode, aber solange es endlich workt, passt das. Außer du willst es dir ansehen und mir vielleicht vereinfachen helfen. Wenn du keine Lust hast, dann passt es auch xD

Achso, also Priorität liegt immer zwischen 0 und 5, wobei 5 die höchste ist?

Jap und mein setter in der Song Klasse, der auch im Konstruktor ist, garantiert das auch. Macht das einen Unterschied ? Ich hab leider immernoch keine Ahnung, inwiefern ich berücksichtigen muss, dass laufende Lieder nicht unterbrochen werden

 

[mrBrown]

Ich hab leider immernoch keine Ahnung, inwiefern ich berücksichtigen muss, dass laufende Lieder nicht unterbrochen werden

Du musst jetzt nur noch dafür sorgen, dass das aktuell gespielte Lied "höher bewertet" wird als eines mit Priorität 5.

Entweder indem du die compareTo Methode in Song überschreibst, und das aktuell gespielte ist einfach immer "größer".
Oder indem du eine "künstliche" Priorität 6 (oder Integer.MAX_VALUE) einführst, die das aktuelle Lied jeweils bekommt.
Oder irgendetwas anderes, Möglichkeiten gäb's viele

 

[xImJugo]

Du musst jetzt nur noch dafür sorgen, dass das aktuell gespielte Lied "höher bewertet" wird als eines mit Priorität 5.

Entweder indem du die compareTo Methode in Song überschreibst, und das aktuell gespielte ist einfach immer "größer".
Oder indem du eine "künstliche" Priorität 6 (oder Integer.MAX_VALUE) einführst, die das aktuelle Lied jeweils bekommt.
Oder irgendetwas anderes, Möglichkeiten gäb's viele

Priorität 0 ist die höchste, aber ich versteh was du meinst. Langt es dann dem abzuspielenden Song auch die 0 zu geben oder geht/muss -1 ?

 

[mrBrown]

Priorität 0 ist die höchste, aber ich versteh was du meinst. Langt es dann dem abzuspielenden Song auch die 0 zu geben oder geht/muss -1 ?

-1 ist sinnvoller