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.
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 ?.let
s, 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()
//...
}