sort: rework implementation

Rework the sort implementation to allow Auxio to leverage it's sorting
capabilities in a more powerful manner.

This is mostly the removal of stupid redunant methods and the change of
Sort overrides to sort in-place. This just gives us the option to avoid
full blown list copies in cases where such is reasonable.

app/src/main/java/org/oxycblt/auxio/detail/DetailViewModel.kt

@@ -116,36 +116,21 @@ class DetailViewModel : ViewModel() {
         refreshGenreData(genre)
     }
 
-    private fun refreshGenreData(genre: Genre) {
-        logD("Refreshing genre data")
-        val data = mutableListOf<Item>(genre)
-        data.add(SortHeader(-2, R.string.lbl_songs))
-        data.addAll(genreSort.genre(genre))
-        _genreData.value = data
-    }
-
-    private fun refreshArtistData(artist: Artist) {
-        logD("Refreshing artist data")
-        val data = mutableListOf<Item>(artist)
-        data.add(Header(-2, R.string.lbl_albums))
-        data.addAll(Sort.ByYear(false).albums(artist.albums))
-        data.add(SortHeader(-3, R.string.lbl_songs))
-        data.addAll(artistSort.artist(artist))
-        _artistData.value = data.toList()
-    }
-
     private fun refreshAlbumData(album: Album) {
         logD("Refreshing album data")
         val data = mutableListOf<Item>(album)
         data.add(SortHeader(id = -2, R.string.lbl_songs))
 
-        val songs = albumSort.album(album)
+        // To create a good user experience regarding disc numbers, we intersperse
+        // items that show the disc number throughout the album's songs. In the case
+        // that the  album does not have disc numbers, we omit the header.
+        val songs = albumSort.songs(album.songs)
         val byDisc = songs.groupBy { it.disc ?: 1 }
         if (byDisc.size > 1) {
             for (entry in byDisc.entries) {
                 val disc = entry.key
                 val discSongs = entry.value
-                data.add(DiscHeader(id = -2L - disc, disc))
+                data.add(DiscHeader(id = -2L - disc, disc)) // Ensure ID uniqueness
                 data.addAll(discSongs)
             }
         } else {
@@ -154,4 +139,22 @@ class DetailViewModel : ViewModel() {
 
         _albumData.value = data
     }
+
+    private fun refreshArtistData(artist: Artist) {
+        logD("Refreshing artist data")
+        val data = mutableListOf<Item>(artist)
+        data.add(Header(-2, R.string.lbl_albums))
+        data.addAll(Sort.ByYear(false).albums(artist.albums))
+        data.add(SortHeader(-3, R.string.lbl_songs))
+        data.addAll(artistSort.songs(artist.songs))
+        _artistData.value = data.toList()
+    }
+
+    private fun refreshGenreData(genre: Genre) {
+        logD("Refreshing genre data")
+        val data = mutableListOf<Item>(genre)
+        data.add(SortHeader(-2, R.string.lbl_songs))
+        data.addAll(genreSort.songs(genre.songs))
+        _genreData.value = data
+    }
 }

app/src/main/java/org/oxycblt/auxio/music/Music.kt

@@ -15,14 +15,7 @@
  * along with this program.  If not, see <https://www.gnu.org/licenses/>.
  */
  
-@file:Suppress(
+@file:Suppress("PropertyName", "FunctionName")
-    "PropertyName",
-    "PropertyName",
-    "PropertyName",
-    "PropertyName",
-    "PropertyName",
-    "PropertyName",
-    "PropertyName")
 
 package org.oxycblt.auxio.music
 
@@ -57,7 +50,9 @@ sealed class Music : Item() {
  * [Music] variant that denotes that this object is a parent of other data objects, such as an
  * [Album] or [Artist]
  */
-sealed class MusicParent : Music()
+sealed class MusicParent : Music() {
+    abstract val songs: List<Song>
+}
 
 /** The data object for a song. */
 data class Song(
@@ -171,7 +166,7 @@ data class Album(
     /** The URI for the cover art corresponding to this album. */
     val albumCoverUri: Uri,
     /** The songs of this album. */
-    val songs: List<Song>,
+    override val songs: List<Song>,
     /** Internal field. Do not use. */
     val _artistGroupingName: String,
 ) : MusicParent() {
@@ -241,11 +236,11 @@ data class Artist(
     override fun resolveName(context: Context) = rawName ?: context.getString(R.string.def_artist)
 
     /** The songs of this artist. */
-    val songs = albums.flatMap { it.songs }
+    override val songs = albums.flatMap { it.songs }
 }
 
 /** The data object for a genre. */
-data class Genre(override val rawName: String?, val songs: List<Song>) : MusicParent() {
+data class Genre(override val rawName: String?, override val songs: List<Song>) : MusicParent() {
     init {
         for (song in songs) {
             song._linkGenre(this)

app/src/main/java/org/oxycblt/auxio/playback/PlaybackViewModel.kt

@@ -226,7 +226,7 @@ class PlaybackViewModel : ViewModel(), PlaybackStateManager.Callback {
 
     /** Add an [Album] to the top of the queue. */
     fun playNext(album: Album) {
-        playbackManager.playNext(settingsManager.detailAlbumSort.album(album))
+        playbackManager.playNext(settingsManager.detailAlbumSort.songs(album.songs))
     }
 
     /** Add a [Song] to the end of the queue. */
@@ -236,7 +236,7 @@ class PlaybackViewModel : ViewModel(), PlaybackStateManager.Callback {
 
     /** Add an [Album] to the end of the queue. */
     fun addToQueue(album: Album) {
-        playbackManager.addToQueue(settingsManager.detailAlbumSort.album(album))
+        playbackManager.addToQueue(settingsManager.detailAlbumSort.songs(album.songs))
     }
 
     // --- STATUS FUNCTIONS ---

app/src/main/java/org/oxycblt/auxio/playback/state/PlaybackStateManager.kt

@@ -124,7 +124,7 @@ class PlaybackStateManager private constructor() {
                 PlaybackMode.IN_GENRE -> song.genre
             }
 
-        applyNewQueue(library, settingsManager.keepShuffle && isShuffled, song, true)
+        applyNewQueue(library, settingsManager.keepShuffle && isShuffled, song)
         notifyNewPlayback()
         notifyShuffledChanged()
         seekTo(0)
@@ -136,10 +136,10 @@ class PlaybackStateManager private constructor() {
      * Play a [parent], such as an artist or album.
      * @param shuffled Whether the queue is shuffled or not
      */
-    fun play(parent: MusicParent?, shuffled: Boolean) {
+    fun play(parent: MusicParent, shuffled: Boolean) {
         val library = musicStore.library ?: return
         this.parent = parent
-        applyNewQueue(library, shuffled, null, true)
+        applyNewQueue(library, shuffled, null)
         notifyNewPlayback()
         notifyShuffledChanged()
         seekTo(0)
@@ -151,7 +151,7 @@ class PlaybackStateManager private constructor() {
     fun shuffleAll() {
         val library = musicStore.library ?: return
         parent = null
-        applyNewQueue(library, true, null, true)
+        applyNewQueue(library, true, null)
         notifyNewPlayback()
         notifyShuffledChanged()
         seekTo(0)
@@ -232,55 +232,41 @@ class PlaybackStateManager private constructor() {
     fun reshuffle(shuffled: Boolean) {
         val library = musicStore.library ?: return
         val song = song ?: return
-        applyNewQueue(library, shuffled, song, false)
+        applyNewQueue(library, shuffled, song)
         notifyQueueChanged()
         notifyShuffledChanged()
     }
 
-    private fun applyNewQueue(
+    private fun applyNewQueue(library: MusicStore.Library, shuffled: Boolean, keep: Song?) {
-        library: MusicStore.Library,
+        val newQueue = (parent?.songs ?: library.songs).toMutableList()
-        shuffled: Boolean,
+        val newIndex: Int
-        keep: Song?,
-        regenShuffledQueue: Boolean
-    ) {
-        if (shuffled) {
-            if (regenShuffledQueue) {
-                _queue =
-                    parent
-                        .let { parent ->
-                            when (parent) {
-                                null -> library.songs
-                                is Album -> parent.songs
-                                is Artist -> parent.songs
-                                is Genre -> parent.songs
-                            }
-                        }
-                        .toMutableList()
-            }
 
-            _queue.shuffle()
+        if (shuffled) {
+            newQueue.shuffle()
 
             if (keep != null) {
-                _queue.add(0, _queue.removeAt(_queue.indexOf(keep)))
+                newQueue.add(0, newQueue.removeAt(newQueue.indexOf(keep)))
             }
 
-            index = 0
+            newIndex = 0
         } else {
-            _queue =
+            val sort =
-                parent
+                parent.let { parent ->
-                    .let { parent ->
+                    when (parent) {
-                        when (parent) {
+                        null -> settingsManager.libSongSort
-                            null -> settingsManager.libSongSort.songs(library.songs)
+                        is Album -> settingsManager.detailAlbumSort
-                            is Album -> settingsManager.detailAlbumSort.album(parent)
+                        is Artist -> settingsManager.detailArtistSort
-                            is Artist -> settingsManager.detailArtistSort.artist(parent)
+                        is Genre -> settingsManager.detailGenreSort
-                            is Genre -> settingsManager.detailGenreSort.genre(parent)
-                        }
                     }
-                    .toMutableList()
+                }
 
-            index = keep?.let(queue::indexOf) ?: 0
+            sort.songsInPlace(newQueue)
+
+            newIndex = keep?.let(queue::indexOf) ?: 0
         }
 
+        _queue = newQueue
+        index = newIndex
         isShuffled = shuffled
     }
 

app/src/main/java/org/oxycblt/auxio/ui/Sort.kt

@@ -25,15 +25,14 @@ import org.oxycblt.auxio.music.Artist
 import org.oxycblt.auxio.music.Genre
 import org.oxycblt.auxio.music.Music
 import org.oxycblt.auxio.music.Song
-import org.oxycblt.auxio.util.logD
 import org.oxycblt.auxio.util.logW
 
 /**
  * A data class representing the sort modes used in Auxio.
  *
- * Sorting can be done by Name, Artist, Album, or Year. Sorting of names is always case-insensitive
+ * Sorting can be done by Name, Artist, Album, and others. Sorting of names is always
- * and article-aware. Certain datatypes may only support a subset of sorts since certain sorts
+ * case-insensitive and article-aware. Certain datatypes may only support a subset of sorts since
- * cannot be easily applied to them (For Example, [Artist] and [ByYear] or [ByAlbum]).
+ * certain sorts cannot be easily applied to them (For Example, [Artist] and [ByYear] or [ByAlbum]).
  *
  * Internally, sorts are saved as an integer in the following format
  *
@@ -50,24 +49,44 @@ sealed class Sort(open val isAscending: Boolean) {
     protected abstract val sortIntCode: Int
     abstract val itemId: Int
 
-    open fun songs(songs: Collection<Song>): List<Song> {
+    fun songs(songs: Collection<Song>): List<Song> {
+        val mutable = songs.toMutableList()
+        songsInPlace(mutable)
+        return mutable
+    }
+
+    fun albums(albums: Collection<Album>): List<Album> {
+        val mutable = albums.toMutableList()
+        albumsInPlace(mutable)
+        return mutable
+    }
+
+    fun artists(artists: Collection<Artist>): List<Artist> {
+        val mutable = artists.toMutableList()
+        artistsInPlace(mutable)
+        return mutable
+    }
+
+    fun genres(genres: Collection<Genre>): List<Genre> {
+        val mutable = genres.toMutableList()
+        genresInPlace(mutable)
+        return mutable
+    }
+
+    open fun songsInPlace(songs: MutableList<Song>) {
         logW("This sort is not supported for songs")
-        return songs.toList()
     }
 
-    open fun albums(albums: Collection<Album>): List<Album> {
+    open fun albumsInPlace(albums: MutableList<Album>) {
         logW("This sort is not supported for albums")
-        return albums.toList()
     }
 
-    open fun artists(artists: Collection<Artist>): List<Artist> {
+    open fun artistsInPlace(artists: MutableList<Artist>) {
         logW("This sort is not supported for artists")
-        return artists.toList()
     }
 
-    open fun genres(genres: Collection<Genre>): List<Genre> {
+    open fun genresInPlace(genres: MutableList<Genre>) {
         logW("This sort is not supported for genres")
-        return genres.toList()
     }
 
     /**
@@ -84,20 +103,20 @@ sealed class Sort(open val isAscending: Boolean) {
         override val itemId: Int
             get() = R.id.option_sort_name
 
-        override fun songs(songs: Collection<Song>): List<Song> {
+        override fun songsInPlace(songs: MutableList<Song>) {
-            return songs.sortedWith(compareByDynamic(NameComparator()) { it })
+            songs.sortWith(compareByDynamic(NameComparator()) { it })
         }
 
-        override fun albums(albums: Collection<Album>): List<Album> {
+        override fun albumsInPlace(albums: MutableList<Album>) {
-            return albums.sortedWith(compareByDynamic(NameComparator()) { it })
+            albums.sortWith(compareByDynamic(NameComparator()) { it })
         }
 
-        override fun artists(artists: Collection<Artist>): List<Artist> {
+        override fun artistsInPlace(artists: MutableList<Artist>) {
-            return artists.sortedWith(compareByDynamic(NameComparator()) { it })
+            artists.sortWith(compareByDynamic(NameComparator()) { it })
         }
 
-        override fun genres(genres: Collection<Genre>): List<Genre> {
+        override fun genresInPlace(genres: MutableList<Genre>) {
-            return genres.sortedWith(compareByDynamic(NameComparator()) { it })
+            genres.sortWith(compareByDynamic(NameComparator()) { it })
         }
 
         override fun ascending(newIsAscending: Boolean): Sort {
@@ -113,8 +132,8 @@ sealed class Sort(open val isAscending: Boolean) {
         override val itemId: Int
             get() = R.id.option_sort_album
 
-        override fun songs(songs: Collection<Song>): List<Song> {
+        override fun songsInPlace(songs: MutableList<Song>) {
-            return songs.sortedWith(
+            songs.sortWith(
                 MultiComparator(
                     compareByDynamic(NameComparator()) { it.album },
                     compareBy(NullableComparator()) { it.track },
@@ -134,8 +153,8 @@ sealed class Sort(open val isAscending: Boolean) {
         override val itemId: Int
             get() = R.id.option_sort_artist
 
-        override fun songs(songs: Collection<Song>): List<Song> {
+        override fun songsInPlace(songs: MutableList<Song>) {
-            return songs.sortedWith(
+            songs.sortWith(
                 MultiComparator(
                     compareByDynamic(NameComparator()) { it.album.artist },
                     compareByDescending(NullableComparator()) { it.album.year },
@@ -144,8 +163,8 @@ sealed class Sort(open val isAscending: Boolean) {
                     compareBy(NameComparator()) { it }))
         }
 
-        override fun albums(albums: Collection<Album>): List<Album> {
+        override fun albumsInPlace(albums: MutableList<Album>) {
-            return albums.sortedWith(
+            albums.sortWith(
                 MultiComparator(
                     compareByDynamic(NameComparator()) { it.artist },
                     compareByDescending(NullableComparator()) { it.year },
@@ -165,8 +184,8 @@ sealed class Sort(open val isAscending: Boolean) {
         override val itemId: Int
             get() = R.id.option_sort_year
 
-        override fun songs(songs: Collection<Song>): List<Song> {
+        override fun songsInPlace(songs: MutableList<Song>) {
-            return songs.sortedWith(
+            songs.sortWith(
                 MultiComparator(
                     compareByDynamic(NullableComparator()) { it.album.year },
                     compareByDescending(NameComparator()) { it.album },
@@ -174,8 +193,8 @@ sealed class Sort(open val isAscending: Boolean) {
                     compareBy(NameComparator()) { it }))
         }
 
-        override fun albums(albums: Collection<Album>): List<Album> {
+        override fun albumsInPlace(albums: MutableList<Album>) {
-            return albums.sortedWith(
+            albums.sortWith(
                 MultiComparator(
                     compareByDynamic(NullableComparator()) { it.year },
                     compareBy(NameComparator()) { it }))
@@ -198,9 +217,8 @@ sealed class Sort(open val isAscending: Boolean) {
         override val itemId: Int
             get() = R.id.option_sort_disc
 
-        override fun songs(songs: Collection<Song>): List<Song> {
+        override fun songsInPlace(songs: MutableList<Song>) {
-            logD(songs)
+            songs.sortWith(
-            return songs.sortedWith(
                 MultiComparator(
                     compareByDynamic(NullableComparator()) { it.disc },
                     compareBy(NullableComparator()) { it.track },
@@ -223,9 +241,8 @@ sealed class Sort(open val isAscending: Boolean) {
         override val itemId: Int
             get() = R.id.option_sort_track
 
-        override fun songs(songs: Collection<Song>): List<Song> {
+        override fun songsInPlace(songs: MutableList<Song>) {
-            logD(songs)
+            songs.sortWith(
-            return songs.sortedWith(
                 MultiComparator(
                     compareBy(NullableComparator()) { it.disc },
                     compareByDynamic(NullableComparator()) { it.track },
@@ -256,30 +273,6 @@ sealed class Sort(open val isAscending: Boolean) {
         }
     }
 
-    /**
-     * Sort the songs in an album.
-     * @see songs
-     */
-    fun album(album: Album): List<Song> {
-        return songs(album.songs)
-    }
-
-    /**
-     * Sort the songs in an artist.
-     * @see songs
-     */
-    fun artist(artist: Artist): List<Song> {
-        return songs(artist.songs)
-    }
-
-    /**
-     * Sort the songs in a genre.
-     * @see songs
-     */
-    fun genre(genre: Genre): List<Song> {
-        return songs(genre.songs)
-    }
-
     protected inline fun <T : Music, K> compareByDynamic(
         comparator: Comparator<in K>,
         crossinline selector: (T) -> K