Skip to content
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

Remove withContext, refactor duplicate methods #907

Merged
merged 2 commits into from
Feb 28, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Next Next commit
Remove withContext, refactor duplicate methods
  • Loading branch information
dturner committed Feb 27, 2023
commit 7f9b3680722f143df81dcdbe2e7a3e9bd48da5ab
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,16 @@
package com.example.android.architecture.blueprints.todoapp.data.source

import com.example.android.architecture.blueprints.todoapp.data.Task
import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.coroutineScope
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext

/**
* Default implementation of [TasksRepository]. Single entry point for managing tasks' data.
*/
class DefaultTasksRepository(
private val tasksRemoteDataSource: TasksDataSource,
private val tasksLocalDataSource: TasksDataSource,
private val ioDispatcher: CoroutineDispatcher = Dispatchers.IO
) : TasksRepository {

override suspend fun getTasks(forceUpdate: Boolean): List<Task> {
Expand Down Expand Up @@ -97,29 +93,17 @@ class DefaultTasksRepository(
}
}

override suspend fun completeTask(task: Task) {
coroutineScope {
launch { tasksRemoteDataSource.completeTask(task) }
launch { tasksLocalDataSource.completeTask(task) }
}
}

override suspend fun completeTask(taskId: String) {
withContext(ioDispatcher) {
getTaskWithId(taskId)?.let { completeTask(it) }
}
}

override suspend fun activateTask(task: Task) = withContext<Unit>(ioDispatcher) {
coroutineScope {
launch { tasksRemoteDataSource.activateTask(task) }
launch { tasksLocalDataSource.activateTask(task) }
launch { tasksRemoteDataSource.completeTask(taskId) }
dturner marked this conversation as resolved.
Show resolved Hide resolved
launch { tasksLocalDataSource.completeTask(taskId) }
}
}

override suspend fun activateTask(taskId: String) {
withContext(ioDispatcher) {
getTaskWithId(taskId)?.let { activateTask(it) }
coroutineScope {
launch { tasksRemoteDataSource.activateTask(taskId) }
launch { tasksLocalDataSource.activateTask(taskId) }
}
}

Expand All @@ -131,11 +115,9 @@ class DefaultTasksRepository(
}

override suspend fun deleteAllTasks() {
dturner marked this conversation as resolved.
Show resolved Hide resolved
withContext(ioDispatcher) {
coroutineScope {
launch { tasksRemoteDataSource.deleteAllTasks() }
launch { tasksLocalDataSource.deleteAllTasks() }
}
coroutineScope {
launch { tasksRemoteDataSource.deleteAllTasks() }
launch { tasksLocalDataSource.deleteAllTasks() }
}
}

Expand All @@ -145,8 +127,4 @@ class DefaultTasksRepository(
launch { tasksLocalDataSource.deleteTask(taskId) }
}
}

private suspend fun getTaskWithId(id: String): Task? {
return tasksLocalDataSource.getTask(id)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,8 @@ interface TasksDataSource {

suspend fun saveTask(task: Task)

suspend fun completeTask(task: Task)

suspend fun completeTask(taskId: String)

suspend fun activateTask(task: Task)

suspend fun activateTask(taskId: String)

suspend fun clearCompletedTasks()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,8 @@ interface TasksRepository {

suspend fun saveTask(task: Task)

suspend fun completeTask(task: Task)

suspend fun completeTask(taskId: String)

suspend fun activateTask(task: Task)

suspend fun activateTask(taskId: String)

suspend fun clearCompletedTasks()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,12 @@ package com.example.android.architecture.blueprints.todoapp.data.source.local

import com.example.android.architecture.blueprints.todoapp.data.Task
import com.example.android.architecture.blueprints.todoapp.data.source.TasksDataSource
import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.withContext

/**
* Concrete implementation of a data source as a db.
*/
class TasksLocalDataSource internal constructor(
private val tasksDao: TasksDao,
private val ioDispatcher: CoroutineDispatcher = Dispatchers.IO
private val tasksDao: TasksDao
) : TasksDataSource {

override fun getTasksStream() = tasksDao.observeTasks()
Expand All @@ -42,43 +38,25 @@ class TasksLocalDataSource internal constructor(
// NO-OP
}

override suspend fun getTasks(): List<Task> = withContext(ioDispatcher) {
return@withContext tasksDao.getTasks()
}
override suspend fun getTasks(): List<Task> = tasksDao.getTasks()

override suspend fun getTask(taskId: String): Task? = withContext(ioDispatcher) {
return@withContext tasksDao.getTaskById(taskId)
}
override suspend fun getTask(taskId: String): Task? = tasksDao.getTaskById(taskId)

override suspend fun saveTask(task: Task) = withContext(ioDispatcher) {
tasksDao.insertTask(task)
}

override suspend fun completeTask(task: Task) = withContext(ioDispatcher) {
tasksDao.updateCompleted(task.id, true)
}
override suspend fun saveTask(task: Task) = tasksDao.insertTask(task)

override suspend fun completeTask(taskId: String) {
override suspend fun completeTask(taskId: String) =
tasksDao.updateCompleted(taskId, true)
}

override suspend fun activateTask(task: Task) = withContext(ioDispatcher) {
tasksDao.updateCompleted(task.id, false)
}

override suspend fun activateTask(taskId: String) {
override suspend fun activateTask(taskId: String) =
tasksDao.updateCompleted(taskId, false)
}

override suspend fun clearCompletedTasks() = withContext<Unit>(ioDispatcher) {
override suspend fun clearCompletedTasks() {
tasksDao.deleteCompletedTasks()
}

override suspend fun deleteAllTasks() = withContext(ioDispatcher) {
tasksDao.deleteTasks()
}
override suspend fun deleteAllTasks() = tasksDao.deleteTasks()

override suspend fun deleteTask(taskId: String) = withContext<Unit>(ioDispatcher) {
override suspend fun deleteTask(taskId: String) {
tasksDao.deleteTaskById(taskId)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -80,22 +80,16 @@ object TasksRemoteDataSource : TasksDataSource {
TASKS_SERVICE_DATA[task.id] = task
}

override suspend fun completeTask(task: Task) {
val completedTask = Task(task.title, task.description, true, task.id)
TASKS_SERVICE_DATA[task.id] = completedTask
}

override suspend fun completeTask(taskId: String) {
// Not required for the remote data source
}

override suspend fun activateTask(task: Task) {
val activeTask = Task(task.title, task.description, false, task.id)
TASKS_SERVICE_DATA[task.id] = activeTask
TASKS_SERVICE_DATA[taskId]?.apply {
isCompleted = true
dturner marked this conversation as resolved.
Show resolved Hide resolved
}
}

override suspend fun activateTask(taskId: String) {
// Not required for the remote data source
TASKS_SERVICE_DATA[taskId]?.apply {
isCompleted = false
}
}

override suspend fun clearCompletedTasks() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import dagger.hilt.android.qualifiers.ApplicationContext
import dagger.hilt.components.SingletonComponent
import javax.inject.Qualifier
import javax.inject.Singleton
import kotlinx.coroutines.CoroutineDispatcher

@Qualifier
@Retention(AnnotationRetention.RUNTIME)
Expand All @@ -50,9 +49,8 @@ object RepositoryModule {
fun provideTasksRepository(
@RemoteTasksDataSource remoteDataSource: TasksDataSource,
@LocalTasksDataSource localDataSource: TasksDataSource,
@IoDispatcher ioDispatcher: CoroutineDispatcher
): TasksRepository {
return DefaultTasksRepository(remoteDataSource, localDataSource, ioDispatcher)
return DefaultTasksRepository(remoteDataSource, localDataSource)
}
}

Expand All @@ -69,10 +67,9 @@ object DataSourceModule {
@LocalTasksDataSource
@Provides
fun provideTasksLocalDataSource(
database: ToDoDatabase,
@IoDispatcher ioDispatcher: CoroutineDispatcher
database: ToDoDatabase
): TasksDataSource {
return TasksLocalDataSource(database.taskDao(), ioDispatcher)
return TasksLocalDataSource(database.taskDao())
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,10 @@ class TaskDetailViewModel @Inject constructor(
fun setCompleted(completed: Boolean) = viewModelScope.launch {
val task = uiState.value.task ?: return@launch
if (completed) {
tasksRepository.completeTask(task)
tasksRepository.completeTask(task.id)
showSnackbarMessage(R.string.task_marked_complete)
} else {
tasksRepository.activateTask(task)
tasksRepository.activateTask(task.id)
showSnackbarMessage(R.string.task_marked_active)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,10 +113,10 @@ class TasksViewModel @Inject constructor(

fun completeTask(task: Task, completed: Boolean) = viewModelScope.launch {
if (completed) {
tasksRepository.completeTask(task)
tasksRepository.completeTask(task.id)
showSnackbarMessage(R.string.task_marked_complete)
} else {
tasksRepository.activateTask(task)
tasksRepository.activateTask(task.id)
showSnackbarMessage(R.string.task_marked_active)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,33 +86,22 @@ class FakeRepository : TasksRepository {
}
}

override suspend fun completeTask(task: Task) {
val completedTask = Task(task.title, task.description, true, task.id)
override suspend fun completeTask(taskId: String) {
_savedTasks.update { tasks ->
val newTasks = LinkedHashMap<String, Task>(tasks)
newTasks[task.id] = completedTask
newTasks[taskId]?.apply { isCompleted = true }
newTasks
}
}

override suspend fun completeTask(taskId: String) {
// Not required for the remote data source.
throw NotImplementedError()
}

override suspend fun activateTask(task: Task) {
val activeTask = Task(task.title, task.description, false, task.id)
override suspend fun activateTask(taskId: String) {
_savedTasks.update { tasks ->
val newTasks = LinkedHashMap<String, Task>(tasks)
newTasks[task.id] = activeTask
newTasks[taskId]?.apply { isCompleted = false }
newTasks
}
}

override suspend fun activateTask(taskId: String) {
throw NotImplementedError()
}

override suspend fun clearCompletedTasks() {
_savedTasks.update { tasks ->
tasks.filterValues {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import androidx.test.filters.MediumTest
import com.example.android.architecture.blueprints.todoapp.MainCoroutineRule
import com.example.android.architecture.blueprints.todoapp.data.Task
import com.example.android.architecture.blueprints.todoapp.data.source.TasksDataSource
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.test.runTest
import org.hamcrest.CoreMatchers.`is`
Expand Down Expand Up @@ -66,7 +65,7 @@ class TasksLocalDataSourceTest {
.allowMainThreadQueries()
.build()

localDataSource = TasksLocalDataSource(database.taskDao(), Dispatchers.Main)
localDataSource = TasksLocalDataSource(database.taskDao())
}

@After
Expand Down Expand Up @@ -96,7 +95,7 @@ class TasksLocalDataSourceTest {
localDataSource.saveTask(newTask)

// When completed in the persistent repository
localDataSource.completeTask(newTask)
localDataSource.completeTask(newTask.id)
val task = localDataSource.getTask(newTask.id)

// Then the task can be retrieved from the persistent repository and is complete
Expand All @@ -110,7 +109,7 @@ class TasksLocalDataSourceTest {
val newTask = Task("Some title", "Some description", true)
localDataSource.saveTask(newTask)

localDataSource.activateTask(newTask)
localDataSource.activateTask(newTask.id)

// Then the task can be retrieved from the persistent repository and is active
val task = localDataSource.getTask(newTask.id)
Expand All @@ -126,9 +125,9 @@ class TasksLocalDataSourceTest {
val newTask2 = Task("title2")
val newTask3 = Task("title3")
localDataSource.saveTask(newTask1)
localDataSource.completeTask(newTask1)
localDataSource.completeTask(newTask1.id)
localDataSource.saveTask(newTask2)
localDataSource.completeTask(newTask2)
localDataSource.completeTask(newTask2.id)
localDataSource.saveTask(newTask3)
// When completed tasks are cleared in the repository
localDataSource.clearCompletedTasks()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,18 +50,10 @@ object FakeFailingTasksRemoteDataSource : TasksDataSource {
TODO("not implemented")
}

override suspend fun completeTask(task: Task) {
TODO("not implemented")
}

override suspend fun completeTask(taskId: String) {
TODO("not implemented")
}

override suspend fun activateTask(task: Task) {
TODO("not implemented")
}

override suspend fun activateTask(taskId: String) {
TODO("not implemented")
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package com.example.android.architecture.blueprints.todoapp.data.source
import com.example.android.architecture.blueprints.todoapp.MainCoroutineRule
import com.example.android.architecture.blueprints.todoapp.data.Task
import com.google.common.truth.Truth.assertThat
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.test.runTest
import org.junit.Before
Expand Down Expand Up @@ -57,7 +56,7 @@ class DefaultTasksRepositoryTest {
tasksLocalDataSource = FakeDataSource(localTasks.toMutableList())
// Get a reference to the class under test
tasksRepository = DefaultTasksRepository(
tasksRemoteDataSource, tasksLocalDataSource, Dispatchers.Main
tasksRemoteDataSource, tasksLocalDataSource
)
}

Expand All @@ -66,7 +65,7 @@ class DefaultTasksRepositoryTest {
fun getTasks_emptyRepositoryAndUninitializedCache() = runTest {
val emptySource = FakeDataSource()
val tasksRepository = DefaultTasksRepository(
emptySource, emptySource, Dispatchers.Main
emptySource, emptySource
)

assertThat(tasksRepository.getTasks().size).isEqualTo(0)
Expand Down
Loading