After some discussion with @mstavares, decided to revert to just one …

…callback that receives a Result
ULHT-CM-2021-22 · Apr 16, 2023 · a90a091 · a90a091
1 parent 6c00ab4
commit a90a091
Show file tree

Hide file tree

Showing 11 changed files with 77 additions and 89 deletions.
diff --git a/README.md b/README.md
@@ -76,10 +76,11 @@ tests need to run in an emulator, I use the fantastic
          id 'kotlin-parcelize'
       }
 
-* Remote calls receive 3 callbacks (only the first is mandatory):
-  * onFinished - called when the server returned the message and it was ok
-  * onError - called when there was an error communicating with the server or the response was not ok
-  * onLoading - called just before initiating the remote request (useful to show a progress indicator)
+* Remote calls receive a onFinished callback with a kotlin.Result:
+  * isSuccess - called when the server returned the message and it was ok
+  * isFailure - called when there was an error communicating with the server or the response was not ok
+
+* The ViewModel receives two callbacks (onSuccess and onFailure) just to show an alternative to the kotlin.Result used in the remote calls (see previous point)
 
 * Uses suspend functions in retrofit, to simplify code and ease debugging. Requires retrofit 2.6.0+
 

diff --git a/app/src/main/java/pt/ulusofona/cm/lotrcharacters/activities/MainActivity.kt b/app/src/main/java/pt/ulusofona/cm/lotrcharacters/activities/MainActivity.kt
@@ -33,10 +33,15 @@ class MainActivity : AppCompatActivity() {
         super.onStart()
 
         binding.getCharactersBtn.setOnClickListener {
+
+            // show a circular progress indicator
+            binding.getCharactersBtn.visibility = View.INVISIBLE
+            binding.progressBar.visibility = View.VISIBLE
+
             CoroutineScope(Dispatchers.IO).launch {
                 // call getCharacters on the "IO Thread"
                 viewModel.getCharacters(
-                    onFinished =  { charactersUI ->
+                    onSuccess =  { charactersUI ->
                             // process the result in the "Main Thread" since it will change the view
                             CoroutineScope(Dispatchers.Main).launch {
                                 val intent = Intent(this@MainActivity, CharactersListActivity::class.java)
@@ -45,7 +50,7 @@ class MainActivity : AppCompatActivity() {
                                 startActivity(intent)
                             }
                     },
-                    onError = {
+                    onFailure = {
                         CoroutineScope(Dispatchers.Main).launch {
 
                             // show a dialog
@@ -61,13 +66,6 @@ class MainActivity : AppCompatActivity() {
                             binding.getCharactersBtn.visibility = View.VISIBLE
                             binding.progressBar.visibility = View.INVISIBLE
                         }
-
-                    },
-                    onLoading = {  // make sure that changes to the view are done in the Main Thread
-                        CoroutineScope(Dispatchers.Main).launch {
-                            binding.getCharactersBtn.visibility = View.INVISIBLE
-                            binding.progressBar.visibility = View.VISIBLE
-                        }
                     }
                 )
             }

diff --git a/...in/java/pt/ulusofona/cm/lotrcharacters/data/remote/okHttp/LOTRServiceWithOkHttpAndGson.kt b/...in/java/pt/ulusofona/cm/lotrcharacters/data/remote/okHttp/LOTRServiceWithOkHttpAndGson.kt
@@ -12,9 +12,7 @@ import java.io.IOException
 
 class LOTRServiceWithOkHttpAndGson(val baseUrl: String = LOTR_API_BASE_URL, val client: OkHttpClient): LOTR() {
 
-    override fun getCharacters(onFinished: (List<LOTRCharacter>) -> Unit,
-                               onError: ((Exception) -> Unit)?,
-                               onLoading: (() -> Unit)?) {
+    override fun getCharacters(onFinished: (Result<List<LOTRCharacter>>) -> Unit) {
 
         // only used for parsing JSON response
         data class Character(val _id: String, val birth: String,
@@ -28,16 +26,14 @@ class LOTRServiceWithOkHttpAndGson(val baseUrl: String = LOTR_API_BASE_URL, val
             .addHeader("Authorization", "Bearer $LOTR_API_TOKEN")
             .build()
 
-        onLoading?.invoke()
-
         val response: ResponseBody? = client.newCall(request).execute().body
         if (response != null) {
             val responseObj = Gson().fromJson(response.string(), GetCharactersResponse::class.java)
-            onFinished(responseObj.docs.map {
+            onFinished(Result.success(responseObj.docs.map {
                 LOTRCharacter(it._id, it.birth, it.death, it.gender.orEmpty(), it.name)
-            })
+            }))
         } else {
-            onError?.invoke(IOException("response was null"))
+            onFinished(Result.failure(IOException("response was null")))
         }
     }
 }
diff --git a/...a/pt/ulusofona/cm/lotrcharacters/data/remote/okHttp/LOTRServiceWithOkHttpAndJSONObject.kt b/...a/pt/ulusofona/cm/lotrcharacters/data/remote/okHttp/LOTRServiceWithOkHttpAndJSONObject.kt
@@ -1,6 +1,5 @@
 package pt.ulusofona.cm.lotrcharacters.data.remote.okHttp
 
-import com.google.gson.Gson
 import okhttp3.*
 import org.json.JSONArray
 import org.json.JSONObject
@@ -13,36 +12,32 @@ import java.io.IOException
 class LOTRServiceWithOkHttpAndJSONObject(val baseUrl: String = LOTR_API_BASE_URL,
                                          val client: OkHttpClient) : LOTR() {
 
-    override fun getCharacters(onFinished: (List<LOTRCharacter>) -> Unit,
-                               onError: ((Exception) -> Unit)?,
-                               onLoading: (() -> Unit)?) {
+    override fun getCharacters(onFinished: (Result<List<LOTRCharacter>>) -> Unit) {
 
         val request: Request = Request.Builder()
             .url("$baseUrl/character")
             .addHeader("Authorization", "Bearer $LOTR_API_TOKEN")
             .build()
 
-        onLoading?.invoke()
-
         client.newCall(request).enqueue(object : Callback {
             override fun onFailure(call: Call, e: IOException) {
-                onError?.invoke(e)
+                onFinished(Result.failure(e))
             }
 
             override fun onResponse(call: Call, response: Response) {
                 response.apply {
                     if (!response.isSuccessful) {
-                        onError?.invoke(IOException("Unexpected code $response"))
+                        onFinished(Result.failure(IOException("Unexpected code $response")))
                     } else {
                         val body = response.body?.string()
                         if (body != null) {
                             val jsonObject = JSONObject(body)
                             val jsonCharactersList = jsonObject["docs"] as JSONArray
-                            val result = mutableListOf<LOTRCharacter>()
+                            val lotrCharacters = mutableListOf<LOTRCharacter>()
                             for (i in 0 until jsonCharactersList.length()) {
                                 val jsonCharacter = jsonCharactersList[i] as JSONObject
 
-                                result.add(
+                                lotrCharacters.add(
                                     LOTRCharacter(
                                         jsonCharacter["_id"].toString(),
                                         jsonCharacter["birth"].toString(),
@@ -53,7 +48,7 @@ class LOTRServiceWithOkHttpAndJSONObject(val baseUrl: String = LOTR_API_BASE_URL
                                 )
                             }
 
-                            onFinished(result)
+                            onFinished(Result.success(lotrCharacters))
                         }
                     }
 

diff --git a/.../main/java/pt/ulusofona/cm/lotrcharacters/data/remote/retrofit/LOTRServiceWithRetrofit.kt b/.../main/java/pt/ulusofona/cm/lotrcharacters/data/remote/retrofit/LOTRServiceWithRetrofit.kt
@@ -11,22 +11,18 @@ import retrofit2.Retrofit
 class LOTRServiceWithRetrofit(val retrofit: Retrofit,
                               private val ioDispatcher: CoroutineDispatcher = Dispatchers.IO): LOTR() {
 
-    override fun getCharacters(onFinished: (List<LOTRCharacter>) -> Unit,
-                               onError: ((Exception) -> Unit)?,
-                               onLoading: (() -> Unit)?) {
+    override fun getCharacters(onFinished: (Result<List<LOTRCharacter>>) -> Unit) {
 
         CoroutineScope(ioDispatcher).launch {
             val service = retrofit.create(LOTRService::class.java)
 
             try {
-                onLoading?.invoke()
-
                 val responseObj: GetCharactersResponse = service.getCharacters()
-                onFinished(responseObj.docs.map {
+                onFinished(Result.success(responseObj.docs.map {
                     LOTRCharacter(it._id, it.birth, it.death, it.gender.orEmpty(), it.name)
-                })
+                }))
             } catch (e: Exception) {
-                onError?.invoke(e)
+                onFinished(Result.failure(e))
             }
         }
     }

diff --git a/.../pt/ulusofona/cm/lotrcharacters/data/remote/urlConnection/LOTRServiceWithUrlConnection.kt b/.../pt/ulusofona/cm/lotrcharacters/data/remote/urlConnection/LOTRServiceWithUrlConnection.kt
@@ -10,9 +10,7 @@ import java.net.URL
 
 class LOTRServiceWithUrlConnection(private val baseUrl: String = LOTR_API_BASE_URL): LOTR() {
 
-    override fun getCharacters(onFinished: (List<LOTRCharacter>) -> Unit,
-                                onError: ((Exception) -> Unit)?,
-                                onLoading: (() -> Unit)?) {
+    override fun getCharacters(onFinished: (Result<List<LOTRCharacter>>) -> Unit) {
 
         val url = URL("$baseUrl/character")
         val connection = url.openConnection()
@@ -21,11 +19,11 @@ class LOTRServiceWithUrlConnection(private val baseUrl: String = LOTR_API_BASE_U
 
         val jsonObject = JSONObject(response)
         val jsonCharactersList = jsonObject["docs"] as JSONArray
-        val result = mutableListOf<LOTRCharacter>()
+        val lotrCharacters = mutableListOf<LOTRCharacter>()
         for (i in 0 until jsonCharactersList.length()) {
             val jsonCharacter = jsonCharactersList[i] as JSONObject
 
-            result.add(
+            lotrCharacters.add(
                 LOTRCharacter(
                     jsonCharacter["_id"].toString(),
                     jsonCharacter["birth"].toString(),
@@ -37,6 +35,6 @@ class LOTRServiceWithUrlConnection(private val baseUrl: String = LOTR_API_BASE_U
 
         }
 
-        onFinished(result)
+        onFinished(Result.success(lotrCharacters))
     }
 }
diff --git a/app/src/main/java/pt/ulusofona/cm/lotrcharacters/model/LOTR.kt b/app/src/main/java/pt/ulusofona/cm/lotrcharacters/model/LOTR.kt
@@ -1,7 +1,5 @@
 package pt.ulusofona.cm.lotrcharacters.model
 
 abstract class LOTR {
-    abstract fun getCharacters(onFinished: (List<LOTRCharacter>) -> Unit,
-                               onError: ((Exception) -> Unit)? = null,
-                               onLoading: (() -> Unit)? = null)
+    abstract fun getCharacters(onFinished: (Result<List<LOTRCharacter>>) -> Unit)
 }
diff --git a/app/src/main/java/pt/ulusofona/cm/lotrcharacters/ui/viewModels/LOTRViewModel.kt b/app/src/main/java/pt/ulusofona/cm/lotrcharacters/ui/viewModels/LOTRViewModel.kt
@@ -5,11 +5,7 @@ import androidx.annotation.VisibleForTesting
 import androidx.lifecycle.ViewModel
 import okhttp3.OkHttpClient
 import pt.ulusofona.cm.lotrcharacters.LOTR_API_BASE_URL
-import pt.ulusofona.cm.lotrcharacters.data.remote.okHttp.LOTRServiceWithOkHttpAndGson
 import pt.ulusofona.cm.lotrcharacters.data.remote.okHttp.LOTRServiceWithOkHttpAndJSONObject
-import pt.ulusofona.cm.lotrcharacters.data.remote.retrofit.LOTRServiceWithRetrofit
-import pt.ulusofona.cm.lotrcharacters.data.remote.retrofit.RetrofitBuilder
-import pt.ulusofona.cm.lotrcharacters.data.remote.urlConnection.LOTRServiceWithUrlConnection
 import pt.ulusofona.cm.lotrcharacters.model.LOTR
 
 class LOTRViewModel: ViewModel() {
@@ -22,34 +18,36 @@ class LOTRViewModel: ViewModel() {
             LOTRServiceWithOkHttpAndJSONObject(LOTR_API_BASE_URL, OkHttpClient())
 //        LOTRServiceWithRetrofit(RetrofitBuilder.getInstance(LOTR_API_BASE_URL))
 
-    fun getCharacters(onFinished: (ArrayList<CharacterUI>) -> Unit,
-                    onError: (Exception) -> Unit,
-                    onLoading: () -> Unit) {
+    fun getCharacters(onSuccess: (ArrayList<CharacterUI>) -> Unit,
+                      onFailure: (Throwable) -> Unit) {
 
         Log.i("APP", "ViewModel.getCharacters")
 
         // transforms "pure" LOTRCharacters into parcelable UICharacters
         model.getCharacters(
             onFinished = {
-                Log.i("APP", "Received ${it.size} characters from WS")
-                val charactersUI = ArrayList(it.map { character ->
-                    CharacterUI(
-                        character.id,
-                        character.birth,
-                        character.death,
-                        character.gender,
-                        character.name
-                    )
-                })
-                onFinished(charactersUI)
-            },
-            onError = {
-                Log.i("APP", "Error getting characters from WS: $it")
-                onError(it)
-            },
-            onLoading = {
-                Log.i("APP", "Loading characters from WS...")
-                onLoading()
+                when {
+                    it.isSuccess -> {
+                        val charactersFromModel = it.getOrNull()!!
+                        Log.i("APP", "Received ${charactersFromModel.size} characters from WS")
+                        val charactersUI = ArrayList(charactersFromModel.map { character ->
+                            CharacterUI(
+                                character.id,
+                                character.birth,
+                                character.death,
+                                character.gender,
+                                character.name
+                            )
+                        })
+                        onSuccess(charactersUI)
+                    }
+
+                    it.isFailure -> {
+                        Log.i("APP", "Error getting characters from WS: $it")
+                        onFailure(it.exceptionOrNull()!!)
+                    }
+                }
+
             }
         )
 

diff --git a/app/src/test/java/pt/ulusofona/cm/lotrcharacters/TestLOTRServiceWithOkHttp.kt b/app/src/test/java/pt/ulusofona/cm/lotrcharacters/TestLOTRServiceWithOkHttp.kt
@@ -6,6 +6,7 @@ import okhttp3.mockwebserver.MockResponse
 import okhttp3.mockwebserver.MockWebServer
 import org.junit.After
 import org.junit.Assert.assertEquals
+import org.junit.Assert.assertTrue
 import org.junit.Before
 import org.junit.Ignore
 import org.junit.Test
@@ -25,10 +26,12 @@ class TestLOTRServiceWithOkHttp: BaseMockWebserverTest() {
 
         val client = OkHttpClient()
         val service = LOTRServiceWithOkHttpAndGson(server.url("").toString(), client)
-        service.getCharacters(onFinished =  {
-            assertEquals(933, it.size)
-            assertEquals("Adanel", it[0].name)
-        })
+        service.getCharacters {
+            assertTrue(it.isSuccess)
+            val characters = it.getOrNull()!!
+            assertEquals(933, characters.size)
+            assertEquals("Adanel", characters[0].name)
+        }
     }
 
     /**
@@ -46,10 +49,11 @@ class TestLOTRServiceWithOkHttp: BaseMockWebserverTest() {
         val service = LOTRServiceWithOkHttpAndJSONObject(server.url("").toString(), client)
 
         val result = mutableMapOf<String,List<LOTRCharacter>>()
-        service.getCharacters(onFinished = {
-            result["list"] = it
+        service.getCharacters {
+            assertTrue(it.isSuccess)
+            result["list"] = it.getOrNull()!!
             latch.countDown()  // count--
-        })
+        }
 
         // to wait for the response
         latch.await(1000, TimeUnit.MILLISECONDS) // suspends until count == 0

diff --git a/app/src/test/java/pt/ulusofona/cm/lotrcharacters/TestLOTRServiceWithRetrofit.kt b/app/src/test/java/pt/ulusofona/cm/lotrcharacters/TestLOTRServiceWithRetrofit.kt
@@ -3,6 +3,7 @@ package pt.ulusofona.cm.lotrcharacters
 import kotlinx.coroutines.delay
 import kotlinx.coroutines.runBlocking
 import okhttp3.mockwebserver.MockResponse
+import org.junit.Assert
 import org.junit.Assert.assertEquals
 import org.junit.Ignore
 import org.junit.Test
@@ -29,10 +30,11 @@ class TestLOTRServiceWithRetrofit: BaseMockWebserverTest() {
 
         val service = LOTRServiceWithRetrofit(retrofit)
         val result = mutableMapOf<String,List<LOTRCharacter>>()
-        service.getCharacters(onFinished =  {
-            result["list"] = it
+        service.getCharacters {
+            Assert.assertTrue(it.isSuccess)
+            result["list"] = it.getOrNull()!!
             latch.countDown()  // count--
-        })
+        }
 
         // to wait for the response
         latch.await(1000, TimeUnit.MILLISECONDS) // suspends until count == 0

diff --git a/app/src/test/java/pt/ulusofona/cm/lotrcharacters/TestLOTRServiceWithUrlConnection.kt b/app/src/test/java/pt/ulusofona/cm/lotrcharacters/TestLOTRServiceWithUrlConnection.kt
@@ -19,10 +19,12 @@ class TestLOTRServiceWithUrlConnection: BaseMockWebserverTest() {
 
         // call the constructor with no args, if you want to connect with the real server
         val service = LOTRServiceWithUrlConnection(server.url("").toString())
-        service.getCharacters(onFinished =  {
-            assertEquals(933, it.size)
-            assertEquals("Adanel", it[0].name)
-        })
+        service.getCharacters {
+            assertTrue(it.isSuccess)
+            val characters = it.getOrNull()!!
+            assertEquals(933, characters.size)
+            assertEquals("Adanel", characters[0].name)
+        }
 
 
     }