Kotlin better readability of MutableLiveData Observer

Question

I would like to make my code more readable by simplifying the observers of the MutableLiveData objects. But I'm not really sure how to do that. I thought about moving the contents of each observe method into a separate class, but that would only improve the readability of the fragment and the new class would be confusing again.

private fun HomeViewModel.setupObservers() {
    messages.observe(viewLifecycleOwner) { response ->
        when (response) {
            is Resource.Success -> {
                response.data?.let { messagesResponse ->
                    getMessagesWithImageUrl(chatID, messagesResponse)
                }
            }
            is Resource.Error -> {
                response.message?.let { message ->
                    Log.e(TAG, "An error occurred: $message")
                }
                hideProgressBarLoadMessages()
                hideProgressBarSendMessage()
            }
            is Resource.Loading -> {}
        }
    }
    messagesWithImageUrl.observe(viewLifecycleOwner) {response ->
        when(response) {
            is Resource.Error -> {
                response.message?.let { message ->
                    Log.e(TAG, "An error occurred: $message")
                }
                hideProgressBarLoadMessages()
                hideProgressBarSendMessage()
            }
            is Resource.Loading -> {}
            is Resource.Success -> {
                response.data?.let { messagesResponse ->
                    adapterMessage
                        .differ
                        .submitList(messagesResponse.thread.values.toList())
                    binding.recyclerViewMessages
                        .scrollToPosition(adapterMessage.itemCount-1)
                    hideProgressBarLoadMessages()
                    hideProgressBarSendMessage()
                }
            }
        }
    }
    messageMediaUpload.observe(viewLifecycleOwner) { response ->
        when(response) {
            is Resource.Error -> {
                response.message?.let { message ->
                    Log.e(TAG, "An error occurred: $message")
                }

            }
            is Resource.Loading -> {
                showProgressBarSendMessage()
            }
            is Resource.Success -> {
                response.data?.let {
                    it.metadata?.let { metadata ->
                        metadata.name?.let { imageName ->
                            val content = imageName.dropLast(4)
                            viewModel.sendMessage(
                                POSTMessage(content, media = true),
                                POSTLastMessage(content, media = true, msgRead = true),
                                chatID,
                                receiverID)
                        }
                    }
                }
            }
        }
    }
    username.observe(viewLifecycleOwner) { response ->
        when(response) {
            is Resource.Error -> {
                response.message?.let { message ->
                    Log.e(TAG, "An error occurred: $message")
                }
            }
            is Resource.Loading -> { /* TODO: some loading animation */ }
            is Resource.Success -> {
                response.data?.let { username ->
                    binding.headlineText.text = username
                }
            }
        }
    }
    sendMessage.observe(viewLifecycleOwner) { response ->
        when(response) {
            is Resource.Error -> {
                response.message?.let { message ->
                    Log.e(TAG, "An error occurred: $message")
                }
                hideProgressBarSendMessage()
            }
            is Resource.Loading -> {
                showProgressBarSendMessage()
            }
            is Resource.Success -> {}
        }
    }
    setMessageRead.observe(viewLifecycleOwner) { response ->
        when(response) {
            is Resource.Error -> {
                response.message?.let { message ->
                    Log.e(TAG, "An error occurred: $message")
                }
            }
            is Resource.Loading -> {}
            is Resource.Success -> {}
        }
    }
}

My Resource class looks like this:

sealed class Resource<T>(
val data: T? = null,
val message: String? = null) {
    class Success<T>(data: T) : Resource<T>(data)
    class Error<T>(message: String, data: T? = null) : Resource<T>(data, message)
    class Loading<T> : Resource<T>()
}

If I changed it like this:

sealed interface Resource<T> {
    class Success<T>(val data: T) : Resource<T>
    class Error<T>(val message: String) : Resource<T>
    class Loading<T> : Resource<T>
}

Then I have to modify methods in my ViewModel as follows:

//old Resource    
fun getChats() = viewModelScope.launch {
    chats.postValue(Resource.Loading())
    homeRepository.getChats().collect() {
        it.data?.let { getChats ->
            setChatIDs(getChats)
            getUnreadMessages(getChats)
        }
        chats.postValue(it)
    }
}
//new Resource
private fun getChats() = viewModelScope.launch {
    chats.postValue(Resource.Loading())
    homeRepository.getChats().collect() {
        if(it is Resource.Success) {
            val data = (it as Resource.Success).data
            setChatIDs(data)
            getUnreadMessages(data)
            
        }
        chats.postValue(it)
    }
}

That would make my Oberver methods more readable, but I'm not sure if that would be bad practise in the ViewModel.

Answer 1

You can't really break these out into classes cleanly because it's working with Fragment internals.

But, from looking at your code and seeing all those ?.lets, I think you need a better sealed class/interface implementation.

I'm guessing your sealed class looks something like this:

sealed class Resource(
    val data: String? = null,
    val message: String? = null
) {
    object Loading: Resource()
    class Success(data: String): Resource(data = data)
    class Error(message: String): Resource(message = message)
}

when it should really look like:

sealed interface Resource {
    object Loading: Resource
    data class Success(val data: String): Resource
    data class Error(val message: String): Resource
}

This way, you don't have a useless message property in your Success class or a useless data property in your Error class. But more importantly, message and data are not nullable because they don't need to be--they are only used in the classes where they are relevant.

Then you won't need to use all those ?.let calls, which are making your code really ugly.

Other than that, you could define:

private fun Resource.Error.log() = Log.e(TAG, "An error occurred: $message")

and use that to avoid the code duplication.

And in your when branches, when there is only a single function call, you can change it to a one-liner with no brackets { } surrounding it. For example:

when (response) {
    // ...
    is Resource.Loading -> showProgressBarSendMessage()
    //...
}