Refactor code (no new functionality added)

- Extract ThreadSafeCachedValueHolder.kt as a separate parameterized
  and re-usable class that can hold any "value" object, not just
  Settings.

- Create a ThreadSafeSettingsHolder class that customizes the generic
  class above for use w/ Settings and Android persistence by providing
  a loader lambda.

- Cleaned up the MainActivity.kt so that it can now correctly respond
  to settings change events fired by EventBus.

- Also in MainActivity.kt, clean up the spinner selection code.
  Currently, onCreate() and onSettingsChangedEvent() both end up
  restoring the saved spinner position. This used to result in needless
  calls to persistence to save and fire this event twice!
  - This happens due to the behavior of the Android spinner to fire
    its onItemSelected() method for programmatic as well as
    user initiated changes to its selection!
  - There are other ways of handling this which I didn't use:
    https://stackoverflow.com/a/25070707/2085356

app/src/main/java/com/r3bl/stayawake/MainActivity.kt

@@ -34,14 +34,13 @@ import androidx.appcompat.app.AppCompatActivity
 import com.r3bl.stayawake.MyTileService.Companion.TAG
 import com.r3bl.stayawake.MyTileServiceSettings.changeSettings
 import kotlinx.android.synthetic.main.activity_main.*
-import kotlinx.android.synthetic.main.activity_main.view.*
 import org.greenrobot.eventbus.EventBus
 import org.greenrobot.eventbus.Subscribe
 import org.greenrobot.eventbus.ThreadMode
 import java.util.concurrent.TimeUnit
 
 /** More info: https://stackoverflow.com/a/25510848/2085356 */
-private class MySpinnerAdapter(context: Context, resource: Int, items: List<String>, private val font: Typeface) :
+class MySpinnerAdapter(context: Context, resource: Int, items: List<String>, private val font: Typeface) :
   ArrayAdapter<String>(context, resource, items) {
   // Affects default (closed) state of the spinner.
   override fun getView(position: Int, convertView: View?, parent: ViewGroup): View =
@@ -57,13 +56,14 @@ class MainActivity : AppCompatActivity() {
   private lateinit var typeNotoSansBold: Typeface
   private lateinit var typeTitilumWebLight: Typeface
   private lateinit var typeTitilumWebRegular: Typeface
-  private lateinit var mySettings: MyTileServiceSettings.ThreadSafeSettingsWrapper
+  private lateinit var mySettingsHolder: ThreadSafeSettingsHolder
+  private lateinit var myAdapter: MySpinnerAdapter
 
   override fun onCreate(savedInstanceState: Bundle?) {
     super.onCreate(savedInstanceState)
     setContentView(R.layout.activity_main)
 
-    mySettings = MyTileServiceSettings.ThreadSafeSettingsWrapper(this)
+    mySettingsHolder = ThreadSafeSettingsHolder(this)
 
     //showAppIconInActionBar();
     //hideStatusBar();
@@ -75,8 +75,9 @@ class MainActivity : AppCompatActivity() {
 
     //handleAutoStartOfService()
 
-    setupCheckbox()
     setupSpinner(typeNotoSansRegular)
+    applyMySettingsHolderValueToCheckbox()
+    applyMySettingsHolderValueToSpinner()
   }
 
   override fun onStart() {
@@ -92,9 +93,13 @@ class MainActivity : AppCompatActivity() {
   /** Handle [SettingsChangedEvent] from [EventBus]. */
   @Subscribe(threadMode = ThreadMode.BACKGROUND)
   fun onSettingsChangedEvent(event: MyTileServiceSettings.SettingsChangedEvent) = event.settings.apply {
-    mySettings.value = this
-    runOnUiThread { formatMessages() }
-    d(TAG, "MainActivity.onSettingsChangedEvent: ${mySettings}")
+    mySettingsHolder.value = this
+    runOnUiThread {
+      formatMessages()
+      applyMySettingsHolderValueToCheckbox()
+      applyMySettingsHolderValueToSpinner()
+    }
+    d(TAG, "MainActivity.onSettingsChangedEvent: ${mySettingsHolder}")
   }
 
 //  private fun handleAutoStartOfService() {
@@ -105,9 +110,30 @@ class MainActivity : AppCompatActivity() {
 //    else d(TAG, "MainActivity.handleAutoStartOfService: Do not auto start")
 //  }
 
-  private fun setupCheckbox() = mySettings.value.autoStartEnabled.let { autoStartEnabled ->
+  private fun applyMySettingsHolderValueToCheckbox() {
+    // Apply mySettings to checkbox_prefs_auto_start.
+    val autoStartEnabled: Boolean = mySettingsHolder.value.autoStartEnabled
     checkbox_prefs_auto_start.isChecked = autoStartEnabled
-    d(TAG, "setupCheckbox: set checkbox state to: $autoStartEnabled")
+    d(TAG, "applyMySettingsToCheckbox: set checkbox state to: $autoStartEnabled")
+  }
+
+  private fun applyMySettingsHolderValueToSpinner() {
+    val spinnerPositionForMySettingsValue: Int = getSpinnerPositionForMySettingsHolderValue()
+    spinner_timeout.setSelection(spinnerPositionForMySettingsValue)
+    d(TAG, "applyMySettingsToSpinner: set spinner selection to position: $spinnerPositionForMySettingsValue")
+
+  }
+
+  /** @return -1 means invalid that the saved value in settings does not match up w/ the currently available options. */
+  private fun getSpinnerPositionForMySettingsHolderValue(): Int {
+    val savedTimeoutInSec: Long = mySettingsHolder.value.timeoutNotChargingSec
+    val savedTimeoutInMin: Long = TimeUnit.MINUTES.convert(savedTimeoutInSec, TimeUnit.SECONDS)
+    return myAdapter.getPosition(savedTimeoutInMin.toString())
+  }
+
+  private fun isSpinnerPositionDifferentThanMySettingsHolderValue(): Boolean {
+    val currentlySelectedSpinnerPosition: Int = spinner_timeout.selectedItemPosition
+    return getSpinnerPositionForMySettingsHolderValue() != currentlySelectedSpinnerPosition
   }
 
   private fun loadAndApplyFonts() {
@@ -142,36 +168,41 @@ class MainActivity : AppCompatActivity() {
         }
   }
 
-  private fun setupSpinner(font: Typeface) = with(spinner_timeout) {
-    // Create custom adatper to handle font.
-    val myAdapter = MySpinnerAdapter(this@MainActivity,
-                                     android.R.layout.simple_spinner_item,
-                                     resources.getStringArray(R.array.spinner_timeout_choices).toList(),
-                                     font)
-    adapter = myAdapter
-
-    // Restore saved selection.
-    val savedTimeoutInSec: Long = mySettings.value.timeoutNotChargingSec
-    val savedTimeoutInMin: Long = TimeUnit.MINUTES.convert(savedTimeoutInSec, TimeUnit.SECONDS)
-    val position = myAdapter.getPosition(savedTimeoutInMin.toString())
-    if (position != -1) {
-      spinner_timeout.setSelection(position)
-      formatMessages()
-      d(TAG, "setupSpinner: set spinner selection to position: $position")
-    }
+  private fun setupSpinner(font: Typeface) {
+    // Create custom adapter to handle font.
+    myAdapter = MySpinnerAdapter(this@MainActivity,
+                                 android.R.layout.simple_spinner_item,
+                                 resources.getStringArray(R.array.spinner_timeout_choices).toList(),
+                                 font)
+    spinner_timeout.adapter = myAdapter
 
     // Attach listeners to handle user selection.
     spinner_timeout.onItemSelectedListener = object : AdapterView.OnItemSelectedListener {
       override fun onNothingSelected(parent: AdapterView<*>?) {}
 
+      /**
+       * This method is called when the [spinner_timeout]'s selected position is set programmatically or by user
+       * interaction.
+       * For programmatic changes, this method filters out the following needless feedback loop:
+       * 1. At startup, spinner index is set programmatically by applyMySettingsToSpinner().
+       * 2. This results in onItemSelected() (this) method to be fired.
+       * 3. The settings are written and an EventBus event is fired
+       * 4. onSettingsChangedEvent() gets called by EventBus, which calls this applyMySettingsToSpinner() (again!)
+       * 5. onItemSelected() (this) method is called (again!)
+       */
       override fun onItemSelected(parent: AdapterView<*>?, view: View?, position: Int, id: Long) {
         val selectionInMin: String = parent?.getItemAtPosition(position).toString()
         val selectionInSec: Long = TimeUnit.SECONDS.convert(selectionInMin.toLong(), TimeUnit.MINUTES)
-        changeSettings(this@MainActivity) {
-          timeoutNotChargingSec = selectionInSec
+
+        if (isSpinnerPositionDifferentThanMySettingsHolderValue()) {
+          d(TAG, "onItemSelected: spinner selection changed to '$selectionInMin' min -> calling changeSettings()")
+          changeSettings(this@MainActivity) {
+            timeoutNotChargingSec = selectionInSec
+          }
+        }
+        else {
+          d(TAG, "onItemSelected: ignore this call since spinner selection is already set to '$selectionInMin' min")
         }
-        formatMessages()
-        d(TAG, "onItemSelected: spinner selection is $selectionInMin")
       }
     }
   }
@@ -213,7 +244,7 @@ class MainActivity : AppCompatActivity() {
   @MainThread
   private fun formatMessages() {
     // Add actual minutes to string template.
-    val hours = TimeUnit.SECONDS.toMinutes(mySettings.value.timeoutNotChargingSec)
+    val hours = TimeUnit.SECONDS.toMinutes(mySettingsHolder.value.timeoutNotChargingSec)
     text_introduction_content.text = getString(R.string.introduction_body, hours)
 
     // Load the Open Source footer as HTML that can be clicked.
@@ -252,5 +283,6 @@ class MainActivity : AppCompatActivity() {
   fun checkboxClicked(view: View) =
       changeSettings(this) {
         autoStartEnabled = (view as CheckBox).isChecked
+        d(TAG, "checkboxClicked: checkbox selection is '$autoStartEnabled' min")
       }
 }

app/src/main/java/com/r3bl/stayawake/MyTileService.kt

@@ -61,13 +61,13 @@ class MyTileService : TileService() {
   private var myIconEyeClosed: Icon? = null
   private var myHandler: Handler? = null
   private var myReceiver: PowerConnectionReceiver? = null
-  private lateinit var mySettings: MyTileServiceSettings.ThreadSafeSettingsWrapper
+  private lateinit var mySettingsHolder: ThreadSafeSettingsHolder
 
   /** Handle [SettingsChangedEvent] from [EventBus]. */
   @Subscribe(threadMode = ThreadMode.BACKGROUND)
   fun onSettingsChangedEvent(event: MyTileServiceSettings.SettingsChangedEvent) {
-    mySettings.value = event.settings
-    d(TAG, "MyTileService.onSettingsChangedEvent: ${mySettings}")
+    mySettingsHolder.value = event.settings
+    d(TAG, "MyTileService.onSettingsChangedEvent: ${mySettingsHolder}")
   }
 
   // General service code.
@@ -77,7 +77,7 @@ class MyTileService : TileService() {
     myIconEyeOpen = Icon.createWithResource(this, R.drawable.ic_stat_visibility)
     myIconEyeClosed = Icon.createWithResource(this, R.drawable.ic_stat_visibility_off)
     myReceiver = PowerConnectionReceiver(this).apply { registerBroadcastReceiver() }
-    mySettings = MyTileServiceSettings.ThreadSafeSettingsWrapper(this)
+    mySettingsHolder = ThreadSafeSettingsHolder(this)
     MyTileServiceSettings.registerWithEventBus(this)
     //handleAutoStartOfService()
     d(TAG, "onCreate: ")
@@ -304,7 +304,7 @@ class MyTileService : TileService() {
     else {
       // Run down the countdown timer.
       myTimeRunning_sec += DELAY_UNIT.convert(DELAY_RECURRING, TimeUnit.SECONDS)
-      if (myTimeRunning_sec >= mySettings.value.timeoutNotChargingSec) {
+      if (myTimeRunning_sec >= mySettingsHolder.value.timeoutNotChargingSec) {
         // Timer has run out.
         if (isCharging(this)) {
           d(TAG, "recurringTask: timer ended but phone is charging")
@@ -360,7 +360,7 @@ class MyTileService : TileService() {
           with(tile) {
             state = Tile.STATE_ACTIVE
             icon = myIconEyeOpen
-            val timeRemaining = mySettings.value.timeoutNotChargingSec - myTimeRunning_sec
+            val timeRemaining = mySettingsHolder.value.timeoutNotChargingSec - myTimeRunning_sec
             val formatTime = formatTime(timeRemaining)
             label = getString(R.string.tile_active_text, formatTime)
           }

app/src/main/java/com/r3bl/stayawake/MyTileServiceSettings.kt

@@ -1,13 +1,12 @@
 package com.r3bl.stayawake
 
 import android.content.Context
+import android.content.SharedPreferences
 import android.preference.PreferenceManager
 import android.util.Log.d
 import com.r3bl.stayawake.MyTileServiceSettings.Settings
 import org.greenrobot.eventbus.EventBus
 import java.util.concurrent.TimeUnit
-import java.util.concurrent.locks.ReentrantReadWriteLock
-import kotlin.concurrent.withLock
 
 /**
  * [Settings] are changed by the user interacting w/ [MainActivity]. These need to be propagated to all the other parts
@@ -19,21 +18,22 @@ import kotlin.concurrent.withLock
  */
 object MyTileServiceSettings {
 
-  // Publicly exposed things.
+  // Settings related classes.
 
   /** Immutable Settings. */
   data class Settings(val autoStartEnabled: Boolean, val timeoutNotChargingSec: Long)
 
-  /** Mutable Settings only used in DSL. */
-  data class MutableSettings(var autoStartEnabled: Boolean? = null, var timeoutNotChargingSec: Long? = null)
+  /** Mutable [Settings] builder only used in DSL. */
+  data class SettingsBuilder(var autoStartEnabled: Boolean? = null, var timeoutNotChargingSec: Long? = null)
 
   /**
-   * DSL that allows [MutableSettings] to be changed, and only changed items are saved to persistence.
+   * DSL that allows [SettingsBuilder] to be changed, and only changed items are saved to persistence.
    */
-  fun changeSettings(context: Context, lambda: MutableSettings.() -> Unit) {
-    val settings = MutableSettings()
-    lambda(settings)
-    saveToPersistence(context, settings)
+  fun changeSettings(context: Context, lambda: SettingsBuilder.() -> Unit) {
+    val settingsBuilder = SettingsBuilder()
+    lambda(settingsBuilder)
+    saveToPersistence(context, settingsBuilder)
+    val settings: Settings = loadFromPersistence(context)
     fireSettingsChangedEvent(settings)
   }
 
@@ -42,14 +42,9 @@ object MyTileServiceSettings {
   /** Wrapper event for use with [EventBus]. */
   data class SettingsChangedEvent(val settings: Settings)
 
-  private fun fireSettingsChangedEvent(settings: MutableSettings) {
-    with(settings) {
-      EventBus.getDefault().post(
-          SettingsChangedEvent(Settings(autoStartEnabled ?: DEFAULT_AUTO_START_ENABLED,
-                                        timeoutNotChargingSec ?: DEFAULT_TIMEOUT_NOT_CHARGING_SEC)
-          ))
-      d(MyTileService.TAG, "fireSettingsChangedEvent, $settings")
-    }
+  private fun fireSettingsChangedEvent(settings: Settings) {
+    EventBus.getDefault().post(SettingsChangedEvent(settings))
+    d(MyTileService.TAG, "fireSettingsChangedEvent, $settings")
   }
 
   /**
@@ -65,51 +60,45 @@ object MyTileServiceSettings {
     EventBus.getDefault().unregister(caller)
   }
 
-  // Thread safe wrapper for [Settings].
-
-  data class ThreadSafeSettingsWrapper(private val myContext: Context) {
-    private val mySettingsLock = ReentrantReadWriteLock()
-    private var _mySettings: Settings? = null
-
-    /** The [value] can be accessed safely from background threads, and the main thread. */
-    var value: Settings
-      set(value) = mySettingsLock.writeLock().withLock {
-        _mySettings = value
-      }
-      get() = mySettingsLock.readLock().withLock {
-        _mySettings ?: loadFromPersistence(myContext).apply { _mySettings = this }
-      }
-
-    override fun toString() = _mySettings?.toString() ?: "Holder.value not set yet"
-  }
-
-  // Hidden internal details.
-
   private var DEFAULT_AUTO_START_ENABLED: Boolean = true
   private var DEFAULT_TIMEOUT_NOT_CHARGING_SEC: Long = TimeUnit.SECONDS.convert(10, TimeUnit.MINUTES)
 
-  private fun loadFromPersistence(context: Context): Settings {
-    val sharedPreferences = PreferenceManager.getDefaultSharedPreferences(context)
-    val immutableSettings = Settings(
-        sharedPreferences.getBoolean(context.getString(R.string.prefs_auto_start_enabled),
-                                     DEFAULT_AUTO_START_ENABLED),
-        sharedPreferences.getLong(context.getString(R.string.prefs_timeout_not_charging_sec),
-                                  DEFAULT_TIMEOUT_NOT_CHARGING_SEC)
+  /** Returns a (immutable) [Settings] object containing the values that have been set, or the defaults. */
+  fun loadFromPersistence(context: Context): Settings {
+    val prefs: SharedPreferences = PreferenceManager.getDefaultSharedPreferences(context)
+    val settings = Settings(
+        prefs.getBoolean(context.getString(R.string.prefs_auto_start_enabled), DEFAULT_AUTO_START_ENABLED),
+        prefs.getLong(context.getString(R.string.prefs_timeout_not_charging_sec), DEFAULT_TIMEOUT_NOT_CHARGING_SEC)
     )
-    d(MyTileService.TAG, "loadSharedPreferences: $immutableSettings")
-    return immutableSettings
+    d(MyTileService.TAG, "loadSharedPreferences: $settings")
+    return settings
   }
 
-  private fun saveToPersistence(context: Context, mutableSettings: MutableSettings) {
-    val sharedPreferencesEdit = PreferenceManager.getDefaultSharedPreferences(context).edit()
+  /** This only saves the fields in [settingsBuilder] that have been set. */
+  fun saveToPersistence(context: Context, settingsBuilder: SettingsBuilder) {
+    val editPrefs = PreferenceManager.getDefaultSharedPreferences(context).edit()
     // Only save fields that have been set.
-    mutableSettings.autoStartEnabled?.let {
-      sharedPreferencesEdit.putBoolean(context.getString(R.string.prefs_auto_start_enabled), it)
+    settingsBuilder.autoStartEnabled?.let {
+      editPrefs.putBoolean(context.getString(R.string.prefs_auto_start_enabled), it)
     }
-    mutableSettings.timeoutNotChargingSec?.let {
-      sharedPreferencesEdit.putLong(context.getString(R.string.prefs_timeout_not_charging_sec), it)
+    settingsBuilder.timeoutNotChargingSec?.let {
+      editPrefs.putLong(context.getString(R.string.prefs_timeout_not_charging_sec), it)
     }
-    d(MyTileService.TAG, "saveSharedPreferences, $mutableSettings")
-    sharedPreferencesEdit.apply()
+    d(MyTileService.TAG, "saveSharedPreferences: $settingsBuilder")
+    editPrefs.apply()
   }
-}
+}
+
+/** Tailored class that uses [ThreadSafeCachedValueHolder] for [Settings] w/ a specific loader. */
+class ThreadSafeSettingsHolder(private val context: Context) {
+  private val myCachedValueHolder: ThreadSafeCachedValueHolder<Settings> =
+      ThreadSafeCachedValueHolder(context) { MyTileServiceSettings.loadFromPersistence(context) }
+
+  var value: Settings
+    get() = myCachedValueHolder.cachedValue
+    set(value) {
+      myCachedValueHolder.cachedValue = value
+    }
+
+  override fun toString(): String = myCachedValueHolder.toString()
+}