Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,183 @@
package ai.elimu.content_provider.ui.letter_sound

import ai.elimu.content_provider.BaseApplication
import ai.elimu.content_provider.R
import ai.elimu.content_provider.rest.LetterSoundsService
import ai.elimu.content_provider.room.GsonToRoomConverter
import ai.elimu.content_provider.room.db.RoomDb
import ai.elimu.content_provider.room.entity.LetterSound_Letter
import ai.elimu.content_provider.room.entity.LetterSound_Sound
import ai.elimu.model.v2.gson.content.LetterSoundGson
import android.os.Bundle
import android.util.Log
import android.view.LayoutInflater
import android.view.View
import android.view.ViewGroup
import android.widget.ProgressBar
import android.widget.TextView
import androidx.fragment.app.Fragment
import androidx.lifecycle.Observer
import androidx.lifecycle.ViewModelProvider
import com.google.android.material.snackbar.Snackbar
import retrofit2.Call
import retrofit2.Callback
import retrofit2.Response
import java.util.concurrent.Executors

class LetterSoundsFragment : Fragment() {
private var letterSoundsViewModel: LetterSoundsViewModel? = null

private var progressBar: ProgressBar? = null

private var textView: TextView? = null

private val TAG = javaClass.name

Comment on lines +27 to +35
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Convert nullable properties to lateinit for better null safety

The Fragment properties are declared as nullable (?) but later accessed with non-null assertions (!!), which defeats Kotlin's null safety benefits and risks runtime crashes.

Consider using lateinit for properties initialized in onCreateView():

-    private var letterSoundsViewModel: LetterSoundsViewModel? = null
-    private var progressBar: ProgressBar? = null
-    private var textView: TextView? = null
+    private lateinit var letterSoundsViewModel: LetterSoundsViewModel
+    private lateinit var progressBar: ProgressBar
+    private lateinit var textView: TextView

Also, use Kotlin's companion object for TAG constant:

-    private val TAG = javaClass.name
+    companion object {
+        private const val TAG = "LetterSoundsFragment"
+    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
class LetterSoundsFragment : Fragment() {
private var letterSoundsViewModel: LetterSoundsViewModel? = null
private var progressBar: ProgressBar? = null
private var textView: TextView? = null
private val TAG = javaClass.name
class LetterSoundsFragment : Fragment() {
private lateinit var letterSoundsViewModel: LetterSoundsViewModel
private lateinit var progressBar: ProgressBar
private lateinit var textView: TextView
companion object {
private const val TAG = "LetterSoundsFragment"
}
// ... (rest of the class)
}

override fun onCreateView(
inflater: LayoutInflater,
container: ViewGroup?,
savedInstanceState: Bundle?
): View? {
Log.i(TAG, "onCreateView")

letterSoundsViewModel = ViewModelProvider(this).get(
LetterSoundsViewModel::class.java
)
val root = inflater.inflate(R.layout.fragment_letter_sounds, container, false)
progressBar = root.findViewById(R.id.progress_bar_letter_sounds)
textView = root.findViewById(R.id.text_letter_sounds) as? TextView
letterSoundsViewModel!!.text.observe(viewLifecycleOwner, object : Observer<String?> {
override fun onChanged(s: String?) {
Log.i(TAG, "onChanged")
textView?.text = s
}
})
return root
}

override fun onStart() {
Log.i(TAG, "onStart")
super.onStart()

// Download LetterSounds from REST API, and store them in the database
val baseApplication = activity!!.application as BaseApplication
val retrofit = baseApplication.retrofit
val letterSoundsService = retrofit.create(
LetterSoundsService::class.java
)
val letterSoundGsonsCall = letterSoundsService.listLetterSounds()
Log.i(TAG, "letterSoundGsonsCall.request(): " + letterSoundGsonsCall.request())
letterSoundGsonsCall.enqueue(object : Callback<List<LetterSoundGson>> {
override fun onResponse(
call: Call<List<LetterSoundGson>>,
response: Response<List<LetterSoundGson>>
) {
Log.i(TAG, "onResponse")

Log.i(TAG, "response: $response")
if (response.isSuccessful) {
val letterSoundGsons = response.body()!!
Log.i(TAG, "letterSoundGsons.size(): " + letterSoundGsons.size)

if (letterSoundGsons.size > 0) {
processResponseBody(letterSoundGsons)
}
} else {
// Handle error
Snackbar.make(textView!!, response.toString(), Snackbar.LENGTH_LONG)
.setBackgroundTint(resources.getColor(R.color.deep_orange_darken_4))
.show()
progressBar!!.visibility = View.GONE
}
}

override fun onFailure(call: Call<List<LetterSoundGson>>, t: Throwable) {
Log.e(TAG, "onFailure", t)

Log.e(TAG, "t.getCause():", t.cause)

// Handle error
Snackbar.make(textView!!, t.cause.toString(), Snackbar.LENGTH_LONG)
.setBackgroundTint(resources.getColor(R.color.deep_orange_darken_4))
.show()
progressBar!!.visibility = View.GONE
}
})
}
Comment on lines +58 to +106
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Avoid force unwrapping with !! and improve API error handling

The code contains multiple force unwraps (!!) which can cause runtime crashes. Additionally, error handling for the API call could be improved.

Replace force unwraps with safer alternatives:

-        val baseApplication = activity!!.application as BaseApplication
+        val baseApplication = requireActivity().application as BaseApplication

For API response handling:

-                    val letterSoundGsons = response.body()!!
+                    val letterSoundGsons = response.body() ?: emptyList()

For error handling:

-                Snackbar.make(textView!!, response.toString(), Snackbar.LENGTH_LONG)
+                Snackbar.make(textView, "Error: ${response.code()} - ${response.message()}", Snackbar.LENGTH_LONG)

And for failure:

-                Snackbar.make(textView!!, t.cause.toString(), Snackbar.LENGTH_LONG)
+                Snackbar.make(textView, "Network error: ${t.message ?: "Unknown error"}", Snackbar.LENGTH_LONG)
+                    .setAction("Retry") { onStart() } // Add retry option
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
override fun onStart() {
Log.i(TAG, "onStart")
super.onStart()
// Download LetterSounds from REST API, and store them in the database
val baseApplication = activity!!.application as BaseApplication
val retrofit = baseApplication.retrofit
val letterSoundsService = retrofit.create(
LetterSoundsService::class.java
)
val letterSoundGsonsCall = letterSoundsService.listLetterSounds()
Log.i(TAG, "letterSoundGsonsCall.request(): " + letterSoundGsonsCall.request())
letterSoundGsonsCall.enqueue(object : Callback<List<LetterSoundGson>> {
override fun onResponse(
call: Call<List<LetterSoundGson>>,
response: Response<List<LetterSoundGson>>
) {
Log.i(TAG, "onResponse")
Log.i(TAG, "response: $response")
if (response.isSuccessful) {
val letterSoundGsons = response.body()!!
Log.i(TAG, "letterSoundGsons.size(): " + letterSoundGsons.size)
if (letterSoundGsons.size > 0) {
processResponseBody(letterSoundGsons)
}
} else {
// Handle error
Snackbar.make(textView!!, response.toString(), Snackbar.LENGTH_LONG)
.setBackgroundTint(resources.getColor(R.color.deep_orange_darken_4))
.show()
progressBar!!.visibility = View.GONE
}
}
override fun onFailure(call: Call<List<LetterSoundGson>>, t: Throwable) {
Log.e(TAG, "onFailure", t)
Log.e(TAG, "t.getCause():", t.cause)
// Handle error
Snackbar.make(textView!!, t.cause.toString(), Snackbar.LENGTH_LONG)
.setBackgroundTint(resources.getColor(R.color.deep_orange_darken_4))
.show()
progressBar!!.visibility = View.GONE
}
})
}
override fun onStart() {
Log.i(TAG, "onStart")
super.onStart()
// Download LetterSounds from REST API, and store them in the database
val baseApplication = requireActivity().application as BaseApplication
val retrofit = baseApplication.retrofit
val letterSoundsService = retrofit.create(
LetterSoundsService::class.java
)
val letterSoundGsonsCall = letterSoundsService.listLetterSounds()
Log.i(TAG, "letterSoundGsonsCall.request(): " + letterSoundGsonsCall.request())
letterSoundGsonsCall.enqueue(object : Callback<List<LetterSoundGson>> {
override fun onResponse(
call: Call<List<LetterSoundGson>>,
response: Response<List<LetterSoundGson>>
) {
Log.i(TAG, "onResponse")
Log.i(TAG, "response: $response")
if (response.isSuccessful) {
val letterSoundGsons = response.body() ?: emptyList()
Log.i(TAG, "letterSoundGsons.size(): " + letterSoundGsons.size)
if (letterSoundGsons.size > 0) {
processResponseBody(letterSoundGsons)
}
} else {
// Handle error
Snackbar.make(textView, "Error: ${response.code()} - ${response.message()}", Snackbar.LENGTH_LONG)
.setBackgroundTint(resources.getColor(R.color.deep_orange_darken_4))
.show()
progressBar!!.visibility = View.GONE
}
}
override fun onFailure(call: Call<List<LetterSoundGson>>, t: Throwable) {
Log.e(TAG, "onFailure", t)
Log.e(TAG, "t.getCause():", t.cause)
// Handle error
Snackbar.make(textView, "Network error: ${t.message ?: "Unknown error"}", Snackbar.LENGTH_LONG)
.setAction("Retry") { onStart() } // Add retry option
.setBackgroundTint(resources.getColor(R.color.deep_orange_darken_4))
.show()
progressBar!!.visibility = View.GONE
}
})
}


private fun processResponseBody(letterSoundGsons: List<LetterSoundGson>) {
Log.i(TAG, "processResponseBody")

val executorService = Executors.newSingleThreadExecutor()
executorService.execute(object : Runnable {
override fun run() {
Log.i(TAG, "run")

val roomDb = RoomDb.getDatabase(context)
val letterSoundDao = roomDb.letterSoundDao()
val letterSound_LetterDao = roomDb.letterSound_LetterDao()
val letterSound_SoundDao = roomDb.letterSound_SoundDao()

// Empty the database table before downloading up-to-date content
letterSound_LetterDao.deleteAll()
letterSound_SoundDao.deleteAll()
letterSoundDao.deleteAll()

for (letterSoundGson in letterSoundGsons) {
Log.i(TAG, "letterSoundGson.getId(): " + letterSoundGson.id)

// Store the LetterSound in the database
val letterSound = GsonToRoomConverter.getLetterSound(letterSoundGson)
letterSoundDao.insert(letterSound)
Log.i(
TAG,
"Stored LetterSound in database with ID " + letterSound.id
)

// Store all the LetterSound's letters in the database
val letterGsons = letterSoundGson.letters
Log.i(TAG, "letterGsons.size(): " + letterGsons.size)
for (letterGson in letterGsons) {
Log.i(TAG, "letterGson.getId(): " + letterGson.id)
val letterSound_Letter = LetterSound_Letter()
letterSound_Letter.letterSound_id = letterSoundGson.id
letterSound_Letter.letters_id = letterGson.id
letterSound_LetterDao.insert(letterSound_Letter)
Log.i(
TAG,
"Stored LetterSound_Letter in database. LetterSound_id: " + letterSound_Letter.letterSound_id + ", letters_id: " + letterSound_Letter.letters_id
)
}

// Store all the LetterSound's sounds in the database
val soundGsons = letterSoundGson.sounds
Log.i(TAG, "soundGsons.size():" + soundGsons.size)
for (soundGson in soundGsons) {
Log.i(TAG, "soundGson.getId(): " + soundGson.id)
val letterSound_Sound = LetterSound_Sound()
letterSound_Sound.letterSound_id = letterSoundGson.id
letterSound_Sound.sounds_id = soundGson.id
letterSound_SoundDao.insert(letterSound_Sound)
Log.i(
TAG,
"Stored LetterSound_Sound in database. LetterSound_id: " + letterSound_Sound.letterSound_id + ", sounds_id: " + letterSound_Sound.sounds_id
)
}
}

// Update the UI
val letterSounds = letterSoundDao.loadAll()
Log.i(TAG, "letterSounds.size(): " + letterSounds.size)
activity!!.runOnUiThread {
textView!!.text = "letterSounds.size(): " + letterSounds.size
Snackbar.make(
textView!!,
"letterSounds.size(): " + letterSounds.size,
Snackbar.LENGTH_LONG
).show()
progressBar!!.visibility = View.GONE
}
}
})
}
Comment on lines +108 to +182
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Replace Java-style background threading with Kotlin Coroutines

The code uses Java's Executors.newSingleThreadExecutor() with anonymous Runnable implementation for background processing, which is verbose and harder to maintain compared to Kotlin coroutines.

Consider refactoring to use Kotlin coroutines:

+ import androidx.lifecycle.lifecycleScope
+ import kotlinx.coroutines.Dispatchers
+ import kotlinx.coroutines.launch
+ import kotlinx.coroutines.withContext

private fun processResponseBody(letterSoundGsons: List<LetterSoundGson>) {
    Log.i(TAG, "processResponseBody")
-    val executorService = Executors.newSingleThreadExecutor()
-    executorService.execute(object : Runnable {
-        override fun run() {
-            // Database code...
-            activity!!.runOnUiThread {
-                // UI updates...
-            }
-        }
-    })
+    lifecycleScope.launch {
+        // Show progress on main thread
+        progressBar.visibility = View.VISIBLE
+        
+        // Do database operations on IO dispatcher
+        withContext(Dispatchers.IO) {
+            // Database code...
+        }
+        
+        // Back on main thread for UI updates
+        textView.text = "letterSounds.size(): $letterSoundsSize"
+        Snackbar.make(
+            textView,
+            "letterSounds.size(): $letterSoundsSize",
+            Snackbar.LENGTH_LONG
+        ).show()
+        progressBar.visibility = View.GONE
+    }
}

This approach:

  1. Uses structured concurrency with coroutines
  2. Properly separates IO and UI operations
  3. Automatically handles context switching
  4. Eliminates force unwrapping
  5. Provides better lifecycle management

Committable suggestion skipped: line range outside the PR's diff.

}