Code review fixes Otus-Android#1

Author: Vladislav Sviridov

app/src/main/java/otus/homework/coroutines/CatViewModel.kt

@@ -1,15 +1,12 @@
 package otus.homework.coroutines
 
-import androidx.lifecycle.LiveData
-import androidx.lifecycle.MutableLiveData
-import androidx.lifecycle.ViewModel
-import androidx.lifecycle.viewModelScope
-import kotlinx.coroutines.*
+import androidx.lifecycle.*
+import kotlinx.coroutines.CoroutineExceptionHandler
+import kotlinx.coroutines.async
+import kotlinx.coroutines.launch
 import java.net.SocketTimeoutException
 
-class CatViewModel(private val catsService: CatsService = DiContainer.service) : ViewModel() {
-
-    private var job: Job? = null
+class CatViewModel(private val catsService: CatsService) : ViewModel() {
 
     private val stateLiveData = MutableLiveData<Result<CatUiState>>()
     val state: LiveData<Result<CatUiState>> = stateLiveData
@@ -23,25 +20,13 @@ class CatViewModel(private val catsService: CatsService = DiContainer.service) :
         CrashMonitor.trackWarning(exception)
     }
 
-    fun onStart() {
-        requestFact()
-    }
-
-    fun onStop() {
-        job?.cancel()
-        job = null
-    }
-
     fun requestFact() {
-        if (job != null && job!!.isActive)
-            return
-
-        job = viewModelScope.launch(handler) {
-            val fact = async(Dispatchers.IO) {
+        viewModelScope.launch(handler) {
+            val fact = async {
                 catsService.getCatFact()
             }
 
-            val image = async(Dispatchers.IO) {
+            val image = async {
                 catsService.getCatImage()
             }
 
@@ -51,3 +36,10 @@ class CatViewModel(private val catsService: CatsService = DiContainer.service) :
         }
     }
 }
+
+@Suppress("UNCHECKED_CAST")
+class CatViewModelFactory(private val catsService: CatsService) : ViewModelProvider.Factory {
+    override fun <T : ViewModel> create(modelClass: Class<T>): T {
+        return CatViewModel(catsService) as T
+    }
+}

app/src/main/java/otus/homework/coroutines/CatsPresenter.kt

@@ -4,60 +4,47 @@ import kotlinx.coroutines.*
 import java.net.SocketTimeoutException
 
 class CatsPresenter(
-    private val catsService: CatsService
+    private val catsService: CatsService,
+    private val presenterScope: CoroutineScope
 ) {
-
-    private var requestJob: Job? = null
     private var _catsView: ICatsView? = null
-
-    private val presenterScope = MainScope() + CoroutineName("CatsCoroutine")
     private val catsView get() = checkNotNull(_catsView)
 
     fun onInitComplete() {
-        loadData()
+        presenterScope.launch {
+            loadData()
+        }
     }
 
     fun attachView(catsView: ICatsView) {
         _catsView = catsView
     }
 
     fun detachView() {
-        presenterScope.cancel()
         _catsView = null
     }
 
-    fun onStart() {
-        loadData()
-    }
-
-    fun onStop() {
-        requestJob?.cancel()
-        requestJob = null
+    fun stop() {
+        presenterScope.cancel()
     }
 
-    private fun loadData() {
-        if (requestJob != null && requestJob!!.isActive)
-            return
-
-        requestJob = presenterScope.launch {
-            try {
-                coroutineScope {
-                    val fact = async(Dispatchers.IO) {
-                        catsService.getCatFact()
-                    }
-
-                    val image = async(Dispatchers.IO) {
-                        catsService.getCatImage()
-                    }
+    private suspend fun loadData() =
+        try {
+            coroutineScope {
+                val fact = async {
+                    catsService.getCatFact()
+                }
 
-                    catsView.populate(CatUiState(fact.await(), image.await()))
+                val image = async {
+                    catsService.getCatImage()
                 }
-            } catch (timeoutException: SocketTimeoutException) {
-                catsView.showMessage("Не удалось получить ответ от сервера")
-            } catch (exception: Exception) {
-                catsView.showMessage("Упс, произошла ошибка: ${exception.message}")
-                CrashMonitor.trackWarning(exception)
+
+                catsView.populate(CatUiState(fact.await(), image.await()))
             }
+        } catch (timeoutException: SocketTimeoutException) {
+            catsView.showMessage("Не удалось получить ответ от сервера")
+        } catch (exception: Exception) {
+            catsView.showMessage("Упс, произошла ошибка: ${exception.message}")
+            CrashMonitor.trackWarning(exception)
         }
-    }
 }

app/src/main/java/otus/homework/coroutines/DiContainer.kt

@@ -1,16 +1,24 @@
 package otus.homework.coroutines
 
+import okhttp3.OkHttpClient
 import retrofit2.Retrofit
 import retrofit2.converter.gson.GsonConverterFactory
 
 object DiContainer {
 
+    private val client = OkHttpClient.Builder().addInterceptor(MockInterceptor()).build()
+
     private val retrofit by lazy {
         Retrofit.Builder()
+            .client(client)
             .baseUrl("https://cat-fact.herokuapp.com/facts/")
             .addConverterFactory(GsonConverterFactory.create())
             .build()
     }
 
     val service by lazy { retrofit.create(CatsService::class.java) }
+
+    val scope by lazy(mode = LazyThreadSafetyMode.NONE) {
+        PresenterScope()
+    }
 }

app/src/main/java/otus/homework/coroutines/MainActivity.kt

@@ -16,7 +16,7 @@ class MainActivity : AppCompatActivity() {
         val view = layoutInflater.inflate(R.layout.activity_main, null) as CatsView
         setContentView(view)
 
-        catsPresenter = CatsPresenter(DiContainer.service)
+        catsPresenter = CatsPresenter(DiContainer.service, DiContainer.scope)
         view.presenter = catsPresenter
         catsPresenter.attachView(view)
         catsPresenter.onInitComplete()
@@ -28,12 +28,7 @@ class MainActivity : AppCompatActivity() {
         }
         super.onStop()
 
-        catsPresenter.onStop()
-    }
-
-    override fun onStart() {
-        super.onStart()
-        catsPresenter.onStart()
+        catsPresenter.stop()
     }
 
     override fun onCreateOptionsMenu(menu: Menu?): Boolean {

app/src/main/java/otus/homework/coroutines/MockInterceptor.kt

@@ -0,0 +1,25 @@
+package otus.homework.coroutines
+
+import okhttp3.*
+
+class MockInterceptor : Interceptor {
+    override fun intercept(chain: Interceptor.Chain): Response {
+        val request = chain.request()
+
+        if (request.url().host() == "cat-fact.herokuapp.com") {
+            val contentType = "application/json"
+            val body = "{\"text\": \"Cats have nine lives\"}"
+
+            return Response.Builder()
+                .request(request)
+                .message(body)
+                .protocol(Protocol.HTTP_1_0)
+                .code(200)
+                .header("content-type", contentType)
+                .body(ResponseBody.create(MediaType.get(contentType), body))
+                .build()
+        }
+
+        return chain.proceed(request)
+    }
+}

app/src/main/java/otus/homework/coroutines/PresenterScope.kt

@@ -0,0 +1,12 @@
+package otus.homework.coroutines
+
+import kotlinx.coroutines.CoroutineName
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.SupervisorJob
+import kotlin.coroutines.CoroutineContext
+
+class PresenterScope : CoroutineScope {
+    override val coroutineContext: CoroutineContext
+        get() = SupervisorJob() + Dispatchers.Main + CoroutineName("CatsCoroutine")
+}

app/src/main/java/otus/homework/coroutines/ViewModelActivity.kt

@@ -6,7 +6,7 @@ import androidx.activity.viewModels
 import androidx.appcompat.app.AppCompatActivity
 
 class ViewModelActivity : AppCompatActivity() {
-    private val viewModel by viewModels<CatViewModel>()
+    private val viewModel by viewModels<CatViewModel> { CatViewModelFactory(DiContainer.service) }
 
     override fun onCreate(savedInstanceState: Bundle?) {
         super.onCreate(savedInstanceState)
@@ -28,14 +28,4 @@ class ViewModelActivity : AppCompatActivity() {
             }
         }
     }
-
-    override fun onStart() {
-        super.onStart()
-        viewModel.onStart()
-    }
-
-    override fun onStop() {
-        super.onStop()
-        viewModel.onStop()
-    }
 }