-
-
Notifications
You must be signed in to change notification settings - Fork 3
Convert LettersFragment from Java to Kotlin #170
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
b8c7f6e
b061699
bef8a71
6313803
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,142 @@ | ||
| package ai.elimu.content_provider.ui.letter | ||
|
|
||
| import ai.elimu.content_provider.BaseApplication | ||
| import ai.elimu.content_provider.R | ||
| import ai.elimu.content_provider.rest.LettersService | ||
| import ai.elimu.content_provider.room.GsonToRoomConverter | ||
| import ai.elimu.content_provider.room.db.RoomDb | ||
| import ai.elimu.model.v2.gson.content.LetterGson | ||
| 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 LettersFragment : Fragment() { | ||
| private var lettersViewModel: LettersViewModel? = null | ||
|
|
||
| private var progressBar: ProgressBar? = null | ||
|
|
||
| private var textView: TextView? = null | ||
|
|
||
| private val TAG = javaClass.name | ||
|
|
||
| override fun onCreateView( | ||
| inflater: LayoutInflater, | ||
| container: ViewGroup?, | ||
| savedInstanceState: Bundle? | ||
| ): View? { | ||
| Log.i(TAG, "onCreateView") | ||
|
|
||
| lettersViewModel = ViewModelProvider(this).get( | ||
| LettersViewModel::class.java | ||
| ) | ||
| val root = inflater.inflate(R.layout.fragment_letters, container, false) | ||
| progressBar = root.findViewById(R.id.progress_bar_letters) | ||
| textView = root.findViewById(R.id.text_letters) as? TextView | ||
| lettersViewModel!!.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 Letters from REST API, and store them in the database | ||
| val baseApplication = activity!!.application as BaseApplication | ||
| val retrofit = baseApplication.retrofit | ||
| val lettersService = retrofit.create(LettersService::class.java) | ||
| val letterGsonsCall = lettersService.listLetters() | ||
| Log.i(TAG, "letterGsonsCall.request(): " + letterGsonsCall.request()) | ||
| letterGsonsCall.enqueue(object : Callback<List<LetterGson>> { | ||
| override fun onResponse( | ||
| call: Call<List<LetterGson>>, | ||
| response: Response<List<LetterGson>> | ||
| ) { | ||
| Log.i(TAG, "onResponse") | ||
|
|
||
| Log.i(TAG, "response: $response") | ||
| if (response.isSuccessful) { | ||
| val letterGsons = response.body()!! | ||
| Log.i(TAG, "letterGsons.size(): " + letterGsons.size) | ||
|
|
||
| if (letterGsons.size > 0) { | ||
| processResponseBody(letterGsons) | ||
| } | ||
| } 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<LetterGson>>, 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 | ||
| } | ||
| }) | ||
| } | ||
|
|
||
| private fun processResponseBody(letterGsons: List<LetterGson>) { | ||
| 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 letterDao = roomDb.letterDao() | ||
|
|
||
| // Empty the database table before downloading up-to-date content | ||
| letterDao.deleteAll() | ||
|
|
||
| for (letterGson in letterGsons) { | ||
| Log.i(TAG, "letterGson.getId(): " + letterGson.id) | ||
|
|
||
| // Store the Letter in the database | ||
| val letter = GsonToRoomConverter.getLetter(letterGson) | ||
| letterDao.insert(letter) | ||
| Log.i(TAG, "Stored Letter in database with ID " + letter.id) | ||
| } | ||
|
|
||
| // Update the UI | ||
| val letters = letterDao.loadAllOrderedByUsageCount() | ||
| Log.i(TAG, "letters.size(): " + letters.size) | ||
| activity!!.runOnUiThread { | ||
| textView!!.text = "letters.size(): " + letters.size | ||
| Snackbar.make( | ||
| textView!!, | ||
| "letters.size(): " + letters.size, | ||
| Snackbar.LENGTH_LONG | ||
| ).show() | ||
| progressBar!!.visibility = View.GONE | ||
| } | ||
| } | ||
| }) | ||
| } | ||
|
Comment on lines
+104
to
+141
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consider using Kotlin Coroutines instead of ExecutorService for background operations. Using Kotlin Coroutines would make the code more concise and easier to manage for asynchronous operations. Also, there are multiple non-null assertions on UI elements that should be handled safely. - val executorService = Executors.newSingleThreadExecutor()
- executorService.execute(object : Runnable {
- override fun run() {
+ viewLifecycleOwner.lifecycleScope.launch(Dispatchers.IO) {
// Database operations...
- activity!!.runOnUiThread {
- textView!!.text = "letters.size(): " + letters.size
- Snackbar.make(
- textView!!,
- "letters.size(): " + letters.size,
- Snackbar.LENGTH_LONG
- ).show()
- progressBar!!.visibility = View.GONE
- }
+ withContext(Dispatchers.Main) {
+ textView?.text = "letters.size(): " + letters.size
+ textView?.let { view ->
+ Snackbar.make(
+ view,
+ "letters.size(): " + letters.size,
+ Snackbar.LENGTH_LONG
+ ).show()
+ }
+ progressBar?.visibility = View.GONE
+ }
- }
- })First, add the necessary imports: import androidx.lifecycle.lifecycleScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.launch
import kotlinx.coroutines.withContextHandle potential null context safely when accessing the database. The code accesses the context without null checking when getting the database instance, which could cause a crash if the fragment is detached. - val roomDb = RoomDb.getDatabase(context)
+ val context = context ?: return@launch // or return@execute if not using coroutines
+ val roomDb = RoomDb.getDatabase(context)
|
||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid using non-null assertions (
!!) and handle activity lifecycle safely.The code uses multiple non-null assertions which could lead to NullPointerExceptions. Also, accessing
activity!!is risky if the fragment is detached.Also, consider handling potential null response body:
And in the onFailure method:
📝 Committable suggestion