“Don’ts” in modern Android/Kotlin Development (Bad practices & Anti-patterns) — Chapter II

Photo by Saif71.com on Unsplash

This series of articles will discuss variety of practices and actions that are likely to decrease your: code quality, product stability, relationships with fellow devs and happiness of your PM/client. I will refer to them as “Dont’s”.

Following “Dont’s” I developed throughout my 4 years journey as Android developer. They will concern various different topics; coding bad practices, anti-patterns that might injury code quality.

Part 1: https://medium.com/@a.poplawski96/donts-in-modern-android-kotlin-development-bad-practices-anti-patterns-chapter-i-d38cba2f5f7d

I also write on “Do’s”, good practices & tips:
Part 1: https://medium.com/@a.poplawski96/dos-in-modern-android-kotlin-development-tips-good-practices-chapter-i-5dfdd276dd6b
Part 2: https://medium.com/@a.poplawski96/dos-in-modern-android-kotlin-development-tips-good-practices-chapter-ii-81b0c0c56667

3. Suspending functions that are not main-safe

As official Android documentation on “Best practices for coroutines in Android” states:

Suspend functions should be main-safe, meaning they’re safe to call from the main thread. If a class is doing long-running blocking operations in a coroutine, it’s in charge of moving the execution off the main thread using withContext. This applies to all classes in your app, regardless of the part of the architecture the class is in.

Let’s consider following example.

class BooksRepository(private val dataSource: BooksDataSource) {

    suspend fun getBooksForAuthorId(authorId: Int): List<Book> {
        /* ... network call implementation using a blocking HttpURLConnection */
    }
}

class AuthorDetailsViewModel(private val booksRepository: BooksRepository) : ViewModel() {

    private val _authorsBooks = MutableStateFlow<List<Book>?>(null)
    val authorsBooks = _authorsBooks.asStateFlow()

    fun fetchBooks(authorId: Int) {
        viewModelScope.launch {
            _authorsBooks.value = booksRepository.getBooksForAuthorId(authorId)
        }
    }
}

With current implementation, making call to booksRepository.getBooksForAuthorId(id) from AuthorDetailsViewModel, without changing dispatchers, results in making a call to a server using HttpURLConnection happening on a main thread — which makes it non main-safe. Let’s fix it.

class BooksRepository(
    private val dataSource: BooksDataSource,
    private val ioDispatcher: CoroutineDispatcher,
) {

    suspend fun getBooksForAuthorId(authorId: Int): List<Book> = withContext(ioDispatcher) {
        return@withContext dataSource.getBooksForAuthorId(authorId)
    }
}

Using withContext(ioDispatcher), we are moving the execution off the main thread, making the function main-safe, since client classes do not need to worry about this.

For more detailed article about main safety, I recommend: https://elizarov.medium.com/blocking-threads-suspending-coroutines-d33e11bf4761

4. Using concrete implementations

Consider following example. We’re working on an application. It’s a real commercial app, and we track analytics using Firebase.

@Composable
fun ExampleScreen(
    viewModel: ExampleViewModel,
    firebaseAnalytics: FirebaseAnalytics,
) {
    LaunchedEffect(null) {
        firebaseAnalytics.logEvent("screen_view", bundleOf("screen_name" to "example_screen"))
    }

    Button(
        onClick = {
            viewModel.handleClick()
            firebaseAnalytics.logEvent("button_click", bundleOf("screen_name" to "example_screen"))
        }
    ) {
        Text(text = "Click me")
    }
    // ...some more code.
}

Let’s assume we have 15, 30 or more screens in our application.
Let’s assume we track proper analytics throughout the whole application (5, 10 or more events per screen).

Now, let’s assume that requirements changed (shocking, I know).
We have to migrate out of current analytics solution. Either Firebase got too expensive, there is a better solution, or we want to use our new in-house analytics.

If we write code like on the example above, i.e. depending on concrete implementations (Firebase Analytics in this case), then we end up with dozens, few dozens or even hundreds line of to modify (depending on the project size), if we want to switch to new analytics solution.

And that’s not good. We can do better.

Here’s what should be done instead.

class AnalyticsLogger(private val firebaseAnalytics: FirebaseAnalytics) {
    
    fun log(event: String, params: Bundle) {
        firebaseAnalytics.logEvent(event, params)
    }
}

Or…

interface AnalyticsLogger {
    fun log(event: String, params: Bundle)
}

class FirebaseAnalyticsLogger(private val firebaseAnalytics: FirebaseAnalytics) : AnalyticsLogger {
    override fun log(event: String, params: Bundle) {
        firebaseAnalytics.logEvent(event, params)
    }
}

Then…

@Composable
fun ExampleScreen(
    viewModel: ExampleViewModel,
    analyticsLogger: AnalyticsLogger,
) {

  LaunchedEffect(null) {
        analyticsLogger.logEvent("screen_view", bundleOf("screen_name" to "example_screen"))
   }

   Button(
        onClick = {
            viewModel.handleClick()
            analyticsLogger.logEvent("button_click", bundleOf("screen_name" to "example_screen"))
        }
    ) {
        Text(text = "Click me")
    }
    // ...some more code.
}

We used AnalyticsLogger abstraction, instead of FirebaseAnalytics concrete implementation. Now, if we need to switch libraries, we just do it in one place. Much better than dozens or hundreds, isn’t it?

Thanks for reading my friend! I write more about Android, Kotlin and programming itself, so if you’re interested in this topic, consider following and seeing my other articles.

And also, let’s connect!
https://www.linkedin.com/in/apoplawski96/