kotlin passing a mutableList in the onNext or a BehaviorSubject which should not be null

Question

Kotlin 1.3.72, RxJava2

I have the following code and I am trying to avoid using the !! operator, but not sure why it thinks the value is null and I need to use the safe call operator.

Later on I have to use the !! which is bad practice. Why would this be null, as I have declared anything to be nullable types?

class SharedViewModel : ViewModel() {
    private val compositeDisposable = CompositeDisposable()
    private val imageSubject = BehaviorSubject.create<MutableList<Photo>>()
    private val selectedPhotos = MutableLiveData<List<Photo>>()

    init {
        imageSubject.subscribeBy {
            selectedPhotos.value = it
        }.addTo(compositeDisposable)
    }

    fun getSelectedPhotos(): LiveData<List<Photo>> {
        return selectedPhotos
    }

    fun addPhotos(photo: Photo) {
        // Not sure why I need the safe-call operator here
        imageSubject.value?.add(photo)
        // Using the !! is bad practice and would like to avoid it
        imageSubject.onNext(imageSubject.value!!)

        // This is how I am currently handling it, but just wondering why the value would be null when it is not a nullable type?
        if(imageSubject.value != null) {
            imageSubject.onNext(imageSubject.value ?: mutableListOf())
        }
    }
}    

=== UPDATE =====

I have made some changes and updated. My final one uses a let.

fun addPhotos(photo: Photo) {
        imageSubject.value?.add(photo)

        // Original
        imageSubject.onNext(imageSubject.value!!)

        // First Attempt
        if(imageSubject.value != null) {
            imageSubject.onNext(imageSubject.value ?: mutableListOf())
        }

        // Final Attempt
        imageSubject.value?.let {
            imageSubject.onNext(it)
        }
    }

Just another question: Is it good practice to add something into a BehaviourSubject imageSubject.value?.add(photo) and then immediately emit that value using the onNext imageSubject.onNext(it)?

Answer 1

BehaviourSubject can have null values since it is intended to emit default value on subscription(which is the latest value it has inside or null if nothing is present) If you don't want such an approach use PublishSubject.

As for the implementation with current types I would do something like

class SharedViewModel : ViewModel() {
    private val compositeDisposable = CompositeDisposable()
    //It is better to use singular object streams instead of list publish
    private val imageSubject = BehaviorSubject.create<Photo>()
    //It is better to use singular object livedata instead of list post. Collect it into a list in place where you subscribe to it or use approach recommended by @Franz Andel above
    private val _selectedPhotos = MutableLiveData<Photo>()
    //Use this approach instead of explicit getter function
    val selectedPhotos: LiveData<Photo>
        get() = _selectedPhotos

    init {
        imageSubject.subscribeBy {
            _selectedPhotos.postValue(it)
        }.addTo(compositeDisposable)
    }

    fun addPhotos(photo: Photo) {
        imageSubject.onNext(photo)
    }
}    

Some recommendation:

//Do not do it like that with subjects - do not add something to their value directly.
        imageSubject.value?.add(photo)
        // Using the !! is bad practice and would like to avoid it
        imageSubject.onNext(imageSubject.value!!)

To answer how to handle nullable types in the best possible way without !!:

//This piece of code is pointless but it has needed context.
imageSubject.value?.apply{
   imageSubject.onNext(this)
}

Also you may want to use elvis operator:

imageSubject.onNext(imageSubject.value ?: someDefaultValue)

Answer 2

Firstly, !! should not be considered as a bad practice. It's a standard part of language, and it's ok to use it when you find it suitable and adequate for your use case.

As I mentioned in the comments section, you have to deal with nullability on the item emitted by BehaviorSubject. One of the solutions to your problem is wrapping BehaviorSubject into non-nullable version:

class NonNullBehaviorSubject<T : Any> constructor(defaultValue: T) { // (1)
    private val wrappedSubject: BehaviorSubject<T> = BehaviorSubject.createDefault(defaultValue)
    val value: T // (2)
        get() {
            return wrappedSubject.value ?: throw RuntimeException("Value not available!")
        }

    fun onNext(value: T) { // (3)
        wrappedSubject.onNext(value)
    }

    fun hide(): Observable<T> = wrappedSubject.hide()
}

This implementation solves all of the nullability issues because:

1. You must initialize the subject with a default value, and the inner type of this subject can't be nullable (<T: Any>).
2. A value you get from the subject is always non-null. If something terrible happens and value is not present (end of the universe), an exception is thrown.
3. You can't put a null value into the subject since method onNext() takes value bounded to type T.

How to use this:

val subject = NonNullBehaviorSubject(emptyList<Photo>()) // (4)

val newValue = subject.value.toMutableList().apply { // (2)
    add(Photo("https://m.dw.com/image/18463014_101.jpg"))
}.toList()

subject.onNext(newValue) // (3)

subject.hide().subscribe { photos -> // (5)
    // do something with photos
}    

Few last tips:

4. When you work with subjects, always use immutable types since it might bring a lot of misunderstanding. Just imagine what might happen when someone modifies the emitted list.
5. It's good practice to use the method hide() to transform Subject to Observable. Observable produced this way can't be used to add new items to Subject so it's safe to pass it the consumer.

Answer 3

Later on I have to use the !! which is bad practice. Why would this be null, as I have declared anything to be nullable types?

value in BehaviorSubject is nullable, you can check into the Java code as it has @Nullable annotation inside. Just like @skywall said.

This is what making you need to define safe call like ? or bang like !! when accessing BehaviorSubject.value.

Just another question: Is it good practice to add something into a BehaviourSubject imageSubject.value?.add(photo) and then immediately emit that value using the onNext imageSubject.onNext(it)?

By default, BehaviorSubject has a null value. So when you don't set any default value or haven't emitted something on your BehaviorSubject, it will always has a null value.

imageSubject.value returns null, so add method won't be called. Notice that you define the safe call ? before calling add method.

So, in conclusion that two lines of code won't emit anything.

From your comment

Would creating a BehaviorSubject with an initial value solve this issue? Or is there something I can do more in my code to make this safe without the !!?

You can define an initial value for BehaviorSubject like this

val imageSubject: BehaviorSubject<MutableList<Photo>> =
        BehaviorSubject.createDefault(mutableListOf(photo1, photo2))

Here for more information

But, defining default value for BehaviorSubject doesn't make the value becomes non-nullable because it is designed to receive nullable object.

So, in order for you not to care about the safe call or bang, you could do it like this

class SharedViewModel : ViewModel() {
    private val compositeDisposable = CompositeDisposable()
    private val imageSubject = BehaviorSubject.create<MutableList<Photo>>()
    private val selectedPhotos = MutableLiveData<List<Photo>>()
    private val photos = mutableListOf<Photo>() // Add this line

    ...

    fun addPhotos(photo: Photo) {
        photos.add(photo)
        imageSubject.onNext(photos)
    }
}