Clean code

Make some minor changes to the codebase.

app/src/main/AndroidManifest.xml

@@ -9,7 +9,7 @@
     <queries />
 
     <application
-        android:name="org.oxycblt.auxio.AuxioApp"
+        android:name=".AuxioApp"
         android:allowBackup="true"
         android:fullBackupContent="@xml/backup_descriptor"
         android:icon="@mipmap/ic_launcher"

app/src/main/java/org/oxycblt/auxio/coil/CoilUtils.kt

@@ -9,10 +9,8 @@ import androidx.databinding.BindingAdapter
 import coil.Coil
 import coil.request.ImageRequest
 import org.oxycblt.auxio.R
-import org.oxycblt.auxio.logE
 import org.oxycblt.auxio.music.Album
 import org.oxycblt.auxio.music.Artist
-import org.oxycblt.auxio.music.BaseModel
 import org.oxycblt.auxio.music.Genre
 import org.oxycblt.auxio.music.Song
 import org.oxycblt.auxio.settings.SettingsManager
@@ -167,27 +165,31 @@ fun ImageView.bindGenreImage(genre: Genre) {
     Coil.imageLoader(context).enqueue(request)
 }
 
-fun ImageRequest.Builder.doCoverSetup(context: Context, data: BaseModel): ImageRequest.Builder {
+/**
-    if (data is Artist || data is Genre) {
+ * Determine if a high quality or low-quality cover needs to be loaded for a specific [Album]
-        logE("doCoverSetup does not support ${data::class.simpleName}")
+ * @return The same builder that this is applied to
+ */
+private fun ImageRequest.Builder.doCoverSetup(context: Context, data: Album): ImageRequest.Builder {
+    if (settingsManager.useQualityCovers) {
+        fetcher(QualityCoverFetcher(context))
+        data(data.songs[0])
+    } else {
+        data(data.coverUri)
+    }
 
     return this
 }
 
+/**
+ * Determine if a high quality or low-quality cover needs to be loaded for a specific [Song]
+ * @return The same builder that this is applied to
+ */
+private fun ImageRequest.Builder.doCoverSetup(context: Context, data: Song): ImageRequest.Builder {
     if (settingsManager.useQualityCovers) {
         fetcher(QualityCoverFetcher(context))
-
-        if (data is Song) {
         data(data)
-        } else if (data is Album) {
-            data(data.songs[0])
-        }
     } else {
-        if (data is Song) {
         data(data.album.coverUri)
-        } else if (data is Album) {
-            data(data.coverUri)
-        }
     }
 
     return this

app/src/main/java/org/oxycblt/auxio/coil/MosaicFetcher.kt

@@ -16,10 +16,12 @@ import coil.fetch.SourceResult
 import coil.size.Size
 import okio.buffer
 import okio.source
+import java.io.Closeable
 import java.io.InputStream
 
 /**
  * A [Fetcher] that takes multiple cover uris and turns them into a 2x2 mosaic image.
+ * @author OxygenCobalt
  */
 class MosaicFetcher(private val context: Context) : Fetcher<List<Uri>> {
     override suspend fun fetch(
@@ -30,8 +32,7 @@ class MosaicFetcher(private val context: Context) : Fetcher<List<Uri>> {
     ): FetchResult {
         val streams = mutableListOf<InputStream>()
 
-        // Load the streams, the lower-quality MediaStore covers are used simply because using
+        // Load MediaStore streams
-        // the raw ones would make loading far too long. Its not that noticeable either.
         data.forEach {
             val stream: InputStream? = context.contentResolver.openInputStream(it)
 
@@ -69,7 +70,9 @@ class MosaicFetcher(private val context: Context) : Fetcher<List<Uri>> {
 
         // For each stream, create a bitmap scaled to 1/4th of the mosaics combined size
         // and place it on a corner of the canvas.
-        for (stream in streams) {
+        streams.useForEach { stream ->
+            if (y == MOSAIC_BITMAP_SIZE) return@useForEach
+
             val bitmap = Bitmap.createScaledBitmap(
                 BitmapFactory.decodeStream(stream),
                 MOSAIC_BITMAP_INCREMENT,
@@ -84,15 +87,8 @@ class MosaicFetcher(private val context: Context) : Fetcher<List<Uri>> {
             if (x == MOSAIC_BITMAP_SIZE) {
                 x = 0
                 y += MOSAIC_BITMAP_INCREMENT
-
-                if (y == MOSAIC_BITMAP_SIZE) {
-                    break
             }
         }
-        }
-
-        // Close all the streams when done.
-        streams.forEach { it.close() }
 
         return DrawableResult(
             drawable = finalBitmap.toDrawable(context.resources),
@@ -101,6 +97,16 @@ class MosaicFetcher(private val context: Context) : Fetcher<List<Uri>> {
         )
     }
 
+    /**
+     * Iterate through a list of [Closeable]s, running [use] on each.
+     * @param action What to do for each [Closeable]
+     */
+    private fun <T : Closeable, R> List<T>.useForEach(action: (T) -> R) {
+        forEach {
+            it.use(action)
+        }
+    }
+
     override fun key(data: List<Uri>): String = data.toString()
 
     companion object {

app/src/main/java/org/oxycblt/auxio/database/PlaybackStateDatabase.kt

@@ -55,14 +55,14 @@ class PlaybackStateDatabase(context: Context) :
      */
     private fun constructStateTable(command: StringBuilder): StringBuilder {
         command.append("${PlaybackState.COLUMN_ID} LONG PRIMARY KEY,")
-        command.append("${PlaybackState.COLUMN_SONG_NAME} STRING NOT NULL,")
+            .append("${PlaybackState.COLUMN_SONG_NAME} STRING NOT NULL,")
-        command.append("${PlaybackState.COLUMN_POSITION} LONG NOT NULL,")
+            .append("${PlaybackState.COLUMN_POSITION} LONG NOT NULL,")
-        command.append("${PlaybackState.COLUMN_PARENT_NAME} STRING NOT NULL,")
+            .append("${PlaybackState.COLUMN_PARENT_NAME} STRING NOT NULL,")
-        command.append("${PlaybackState.COLUMN_INDEX} INTEGER NOT NULL,")
+            .append("${PlaybackState.COLUMN_INDEX} INTEGER NOT NULL,")
-        command.append("${PlaybackState.COLUMN_MODE} INTEGER NOT NULL,")
+            .append("${PlaybackState.COLUMN_MODE} INTEGER NOT NULL,")
-        command.append("${PlaybackState.COLUMN_IS_SHUFFLING} BOOLEAN NOT NULL,")
+            .append("${PlaybackState.COLUMN_IS_SHUFFLING} BOOLEAN NOT NULL,")
-        command.append("${PlaybackState.COLUMN_LOOP_MODE} INTEGER NOT NULL,")
+            .append("${PlaybackState.COLUMN_LOOP_MODE} INTEGER NOT NULL,")
-        command.append("${PlaybackState.COLUMN_IN_USER_QUEUE} BOOLEAN NOT NULL)")
+            .append("${PlaybackState.COLUMN_IN_USER_QUEUE} BOOLEAN NOT NULL)")
 
         return command
     }
@@ -72,9 +72,9 @@ class PlaybackStateDatabase(context: Context) :
      */
     private fun constructQueueTable(command: StringBuilder): StringBuilder {
         command.append("${QueueItem.COLUMN_ID} LONG PRIMARY KEY,")
-        command.append("${QueueItem.COLUMN_SONG_NAME} LONG NOT NULL,")
+            .append("${QueueItem.COLUMN_SONG_NAME} LONG NOT NULL,")
-        command.append("${QueueItem.COLUMN_ALBUM_NAME} LONG NOT NULL,")
+            .append("${QueueItem.COLUMN_ALBUM_NAME} LONG NOT NULL,")
-        command.append("${QueueItem.COLUMN_IS_USER_QUEUE} BOOLEAN NOT NULL)")
+            .append("${QueueItem.COLUMN_IS_USER_QUEUE} BOOLEAN NOT NULL)")
 
         return command
     }

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

@@ -76,6 +76,7 @@ class AlbumDetailFragment : DetailFragment() {
         detailModel.albumSortMode.observe(viewLifecycleOwner) { mode ->
             logD("Updating sort mode to $mode")
 
+            // Detail header data is included
             val data = mutableListOf<BaseModel>(detailModel.currentAlbum.value!!).also {
                 it.addAll(mode.getSortedSongList(detailModel.currentAlbum.value!!.songs))
             }
@@ -85,8 +86,9 @@ class AlbumDetailFragment : DetailFragment() {
 
         detailModel.navToItem.observe(viewLifecycleOwner) {
             if (it != null) {
-                logD(it.name)
                 when (it) {
+                    // Songs should be scrolled to if the album matches, or a new detail
+                    // fragment should be launched otherwise.
                     is Song -> {
                         if (detailModel.currentAlbum.value!!.id == it.album.id) {
                             scrollToItem(it.id)
@@ -99,6 +101,8 @@ class AlbumDetailFragment : DetailFragment() {
                         }
                     }
 
+                    // If the album matches, no need to do anything. Otherwise launch a new
+                    // detail fragment.
                     is Album -> {
                         if (detailModel.currentAlbum.value!!.id == it.id) {
                             binding.detailRecycler.scrollToPosition(0)
@@ -110,8 +114,8 @@ class AlbumDetailFragment : DetailFragment() {
                         }
                     }
 
+                    // Always launch a new ArtistDetailFragment.
                     is Artist -> {
-                        logD("Hello?")
                         findNavController().navigate(
                             AlbumDetailFragmentDirections.actionShowArtist(it.id)
                         )
@@ -125,7 +129,14 @@ class AlbumDetailFragment : DetailFragment() {
         // --- PLAYBACKVIEWMODEL SETUP ---
 
         playbackModel.song.observe(viewLifecycleOwner) {
-            handlePlayingItem(detailAdapter)
+            if (playbackModel.mode.value == PlaybackMode.IN_ALBUM &&
+                playbackModel.parent.value?.id == detailModel.currentAlbum.value!!.id
+            ) {
+                detailAdapter.highlightSong(playbackModel.song.value, binding.detailRecycler)
+            } else {
+                // Clear the viewholders if the mode isn't ALL_SONGS
+                detailAdapter.highlightSong(null, binding.detailRecycler)
+            }
         }
 
         playbackModel.isInUserQueue.observe(viewLifecycleOwner) {
@@ -139,21 +150,6 @@ class AlbumDetailFragment : DetailFragment() {
         return binding.root
     }
 
-    /**
-     * Handle an update to the mode or the song and determine whether to highlight a song
-     * item based off that
-     */
-    private fun handlePlayingItem(detailAdapter: AlbumDetailAdapter) {
-        if (playbackModel.mode.value == PlaybackMode.IN_ALBUM &&
-            playbackModel.parent.value?.id == detailModel.currentAlbum.value!!.id
-        ) {
-            detailAdapter.highlightSong(playbackModel.song.value, binding.detailRecycler)
-        } else {
-            // Clear the viewholders if the mode isn't ALL_SONGS
-            detailAdapter.highlightSong(null, binding.detailRecycler)
-        }
-    }
-
     private fun scrollToItem(id: Long) {
         // Calculate where the item for the currently played song is
         val pos = detailModel.albumSortMode.value!!.getSortedSongList(

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

@@ -11,6 +11,7 @@ import org.oxycblt.auxio.logD
 import org.oxycblt.auxio.music.Album
 import org.oxycblt.auxio.music.Artist
 import org.oxycblt.auxio.music.BaseModel
+import org.oxycblt.auxio.music.Genre
 import org.oxycblt.auxio.music.MusicStore
 import org.oxycblt.auxio.music.Song
 import org.oxycblt.auxio.playback.state.PlaybackMode
@@ -70,6 +71,7 @@ class ArtistDetailFragment : DetailFragment() {
         detailModel.artistSortMode.observe(viewLifecycleOwner) { mode ->
             logD("Updating sort mode to $mode")
 
+            // Header detail data is always included
             val data = mutableListOf<BaseModel>(detailModel.currentArtist.value!!).also {
                 it.addAll(mode.getSortedAlbumList(detailModel.currentArtist.value!!.albums))
             }
@@ -79,15 +81,18 @@ class ArtistDetailFragment : DetailFragment() {
 
         detailModel.navToItem.observe(viewLifecycleOwner) {
             if (it != null) {
+                // If the artist matches, no need to do anything, otherwise launch a new detail
                 if (it is Artist) {
                     if (it.id == detailModel.currentArtist.value!!.id) {
+                        binding.detailRecycler.scrollToPosition(0)
                         detailModel.doneWithNavToItem()
                     } else {
                         findNavController().navigate(
                             ArtistDetailFragmentDirections.actionShowArtist(it.id)
                         )
                     }
-                } else {
+                } else if (it !is Genre) {
+                    // Determine the album id of the song or album, and then launch it otherwise
                     val albumId = if (it is Song) it.album.id else it.id
 
                     findNavController().navigate(
@@ -97,6 +102,7 @@ class ArtistDetailFragment : DetailFragment() {
             }
         }
 
+        // Highlight albums if they are being played
         playbackModel.parent.observe(viewLifecycleOwner) { parent ->
             if (playbackModel.mode.value == PlaybackMode.IN_ALBUM && parent is Album?) {
                 detailAdapter.setCurrentAlbum(parent, binding.detailRecycler)

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

@@ -83,6 +83,7 @@ abstract class DetailFragment : Fragment() {
             adapter = detailAdapter
             setHasFixedSize(true)
 
+            // Set up a grid if the mode is landscape
             if (isLandscape(resources)) {
                 layoutManager = GridLayoutManager(requireContext(), 2).also {
                     it.spanSizeLookup = object : GridLayoutManager.SpanSizeLookup() {
@@ -93,6 +94,8 @@ abstract class DetailFragment : Fragment() {
                 }
             }
 
+            // Since there is no elevation when the scroll position is zero, dont show
+            // the overscroll indicator.
             addOnScrollListener(object : RecyclerView.OnScrollListener() {
                 override fun onScrolled(recyclerView: RecyclerView, dx: Int, dy: Int) {
                     overScrollMode = if (computeVerticalScrollOffset() == 0) {

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

@@ -63,6 +63,7 @@ class GenreDetailFragment : DetailFragment() {
         detailModel.genreSortMode.observe(viewLifecycleOwner) { mode ->
             logD("Updating sort mode to $mode")
 
+            // Detail header data is included
             val data = mutableListOf<BaseModel>(detailModel.currentGenre.value!!).also {
                 it.addAll(mode.getSortedSongList(detailModel.currentGenre.value!!.songs))
             }
@@ -73,6 +74,7 @@ class GenreDetailFragment : DetailFragment() {
         detailModel.navToItem.observe(viewLifecycleOwner) {
             if (it != null) {
                 when (it) {
+                    // All items will launch new detail fragments.
                     is Artist -> findNavController().navigate(
                         GenreDetailFragmentDirections.actionShowArtist(it.id)
                     )
@@ -93,7 +95,14 @@ class GenreDetailFragment : DetailFragment() {
         // --- PLAYBACKVIEWMODEL SETUP ---
 
         playbackModel.song.observe(viewLifecycleOwner) {
-            handlePlayingItem(detailAdapter)
+            if (playbackModel.mode.value == PlaybackMode.IN_GENRE &&
+                playbackModel.parent.value?.id == detailModel.currentGenre.value!!.id
+            ) {
+                detailAdapter.highlightSong(playbackModel.song.value, binding.detailRecycler)
+            } else {
+                // Clear the viewholders if the mode isn't ALL_SONGS
+                detailAdapter.highlightSong(null, binding.detailRecycler)
+            }
         }
 
         playbackModel.isInUserQueue.observe(viewLifecycleOwner) {
@@ -106,19 +115,4 @@ class GenreDetailFragment : DetailFragment() {
 
         return binding.root
     }
-
-    /**
-     * Handle an update to the mode or the song and determine whether to highlight a song
-     * item based off that
-     */
-    private fun handlePlayingItem(detailAdapter: GenreDetailAdapter) {
-        if (playbackModel.mode.value == PlaybackMode.IN_GENRE &&
-            playbackModel.parent.value?.id == detailModel.currentGenre.value!!.id
-        ) {
-            detailAdapter.highlightSong(playbackModel.song.value, binding.detailRecycler)
-        } else {
-            // Clear the viewholders if the mode isn't ALL_SONGS
-            detailAdapter.highlightSong(null, binding.detailRecycler)
-        }
-    }
 }

app/src/main/java/org/oxycblt/auxio/library/LibraryFragment.kt

@@ -101,11 +101,11 @@ class LibraryFragment : Fragment() {
 
     /**
      * Navigate to an item
-     * @param baseModel The data things should be done with
+     * @param baseModel The item that should be navigated to.
      */
     private fun onItemSelection(baseModel: BaseModel) {
         if (baseModel is Song) {
-            logE("onItemSelection does not support song")
+            logE("onItemSelection does not support songs")
             return
         }
 

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

@@ -46,7 +46,6 @@ class CompactPlaybackFragment : Fragment() {
         // Put a placeholder song in the binding & hide the playback fragment initially.
         binding.song = MusicStore.getInstance().songs[0]
         binding.playbackModel = playbackModel
-
         if (playbackModel.song.value == null && isLandscape) {
             hideAll(binding)
         }

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

@@ -30,8 +30,8 @@ class PlaybackFragment : Fragment(), SeekBar.OnSeekBarChangeListener {
     private val playbackModel: PlaybackViewModel by activityViewModels()
     private val detailModel: DetailViewModel by activityViewModels()
     private val binding: FragmentPlaybackBinding by memberBinding(FragmentPlaybackBinding::inflate) {
-        // Marquee must be disabled on destroy to prevent memory leaks
+        // Marquee must be disabled on destruction to prevent memory leaks
-        binding.playbackSong.isSelected = false
+        playbackSong.isSelected = false
     }
 
     // Colors

app/src/main/java/org/oxycblt/auxio/search/SearchAdapter.kt

@@ -56,7 +56,7 @@ class SearchAdapter(
 
             HeaderViewHolder.ITEM_TYPE -> HeaderViewHolder.from(parent.context)
 
-            else -> error("Someone messed with the ViewHolder item types.")
+            else -> error("Invalid viewholder item type.")
         }
     }
 

app/src/main/java/org/oxycblt/auxio/settings/SettingsListFragment.kt

@@ -63,9 +63,9 @@ class SettingsListFragment : PreferenceFragmentCompat() {
         }
     }
 
-    private fun handlePreference(it: Preference) {
+    private fun handlePreference(pref: Preference) {
-        it.apply {
+        pref.apply {
-            when (it.key) {
+            when (key) {
                 SettingsManager.Keys.KEY_THEME -> {
                     setIcon(AppCompatDelegate.getDefaultNightMode().toThemeIcon())
 

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

@@ -94,7 +94,7 @@ fun Fragment.requireCompatActivity(): AppCompatActivity {
 }
 
 /**
- * "Render" a [Spanned] using [HtmlCompat].
+ * "Render" a [Spanned] using [HtmlCompat]. (As in making text bolded and whatnot).
  * @return A [Spanned] that actually works.
  */
 fun Spanned.render(): Spanned {

app/src/main/res/layout-land/fragment_compact_playback.xml

@@ -44,9 +44,8 @@
             android:layout_height="wrap_content"
             android:layout_marginStart="@dimen/margin_mid_small"
             android:layout_marginEnd="@dimen/margin_mid_small"
-            android:ellipsize="marquee"
             android:fontFamily="@font/inter_semibold"
-            android:marqueeRepeatLimit="marquee_forever"
+            android:ellipsize="end"
             android:singleLine="true"
             android:text="@{song.name}"
             android:textAppearance="@style/TextAppearance.SmallHeader"
@@ -63,8 +62,7 @@
             android:layout_height="wrap_content"
             android:layout_marginStart="@dimen/margin_mid_small"
             android:layout_marginEnd="@dimen/margin_mid_small"
-            android:ellipsize="marquee"
+            android:ellipsize="end"
-            android:marqueeRepeatLimit="marquee_forever"
             android:singleLine="true"
             android:text="@{@string/format_info(song.album.artist.name, song.album.name)}"
             android:textAppearance="@style/TextAppearance.MaterialComponents.Caption"

app/src/main/res/layout/fragment_compact_playback.xml

@@ -53,9 +53,8 @@
             android:layout_height="wrap_content"
             android:layout_marginStart="@dimen/margin_mid_small"
             android:layout_marginEnd="@dimen/margin_mid_small"
-            android:ellipsize="marquee"
+            android:ellipsize="end"
             android:fontFamily="@font/inter_semibold"
-            android:marqueeRepeatLimit="marquee_forever"
             android:singleLine="true"
             android:text="@{song.name}"
             android:textAppearance="@style/TextAppearance.SmallHeader"
@@ -72,8 +71,7 @@
             android:layout_height="wrap_content"
             android:layout_marginStart="@dimen/margin_mid_small"
             android:layout_marginEnd="@dimen/margin_mid_small"
-            android:ellipsize="marquee"
+            android:ellipsize="end"
-            android:marqueeRepeatLimit="marquee_forever"
             android:singleLine="true"
             android:text="@{@string/format_info(song.album.artist.name, song.album.name)}"
             android:textAppearance="@style/TextAppearance.MaterialComponents.Caption"

app/src/main/res/values/do_not_translate.xml

@@ -1,10 +1,10 @@
 <?xml version="1.0" encoding="utf-8"?>
 <resources>
-    <string name="label_author_oxycblt">OxygenCobalt</string>
+    <string name="label_author_oxycblt" translatable="false">OxygenCobalt</string>
 
     <!-- Format Namespace | Value formatting/plurals -->
-    <string name="format_info">%1$s / %2$s</string>
+    <string name="format_info" translatable="false">%1$s / %2$s</string>
-    <string name="format_double_info">%1$s / %2$s / %3$s</string>
+    <string name="format_double_info" translatable="false">%1$s / %2$s / %3$s</string>
-    <string name="format_double_counts">%1$s, %2$s</string>
+    <string name="format_double_counts" translatable="false">%1$s, %2$s</string>
-    <string name="format_accent_summary">&lt;b>%1$s&lt;/b>:&#160;%2$s</string>
+    <string name="format_accent_summary" translatable="false">&lt;b>%1$s&lt;/b>:&#160;%2$s</string>
 </resources>

app/src/main/res/values/strings.xml

@@ -93,7 +93,7 @@
     <string name="setting_behavior_at_end">When a playlist ends</string>
     <string name="setting_behavior_end_loop">Loop</string>
     <string name="setting_behavior_end_stop">Stop</string>
-    <string name="setting_behavior_end_loop_pause">Loop &amp; Pause</string>
+    <string name="setting_behavior_end_loop_pause">Loop and Pause</string>
 
     <string name="setting_behavior_keep_shuffle">Remember shuffle</string>
     <string name="setting_behavior_keep_shuffle_desc">Keep shuffle on when playing a new song</string>
@@ -108,10 +108,10 @@
     <string name="debug_state_saved">State saved</string>
 
     <!-- Error Namespace | Error Labels -->
-    <string name="error_no_music">No music found.</string>
+    <string name="error_no_music">No music found</string>
-    <string name="error_music_load_failed">Music loading failed.</string>
+    <string name="error_music_load_failed">Music loading failed</string>
-    <string name="error_no_perms">Permissions to read storage are needed.</string>
+    <string name="error_no_perms">Permissions to read storage are needed</string>
-    <string name="error_no_browser">Could not open link.</string>
+    <string name="error_no_browser">Could not open link</string>
 
     <!-- Hint Namespace | EditText Hints -->
     <string name="hint_search_library">Search your library…</string>