sockeqwe / AdapterDelegates

"Favor composition over inheritance" for RecyclerView Adapters
http://hannesdorfmann.com/android/adapter-delegates
Apache License 2.0
2.93k stars 313 forks source link

Make getItems() null-safe for Kotlin interoperability #106

Closed Sab44 closed 2 years ago

Sab44 commented 2 years ago

Hi,

we are using a ListDelegationAdapter in our Kotlin project and everything is working nicely.

There is only one thing I would love to have and that is a null-safe access to the items variable declared in the AbsDelegationAdapter. Currently Kotlin does not enforce nullability when calling items (or getItems()).

We can build something like this as a superclass for our adapters:

abstract class BridgedListDelegationAdapter<T : List<Any>> : ListDelegationAdapter<T> {

    constructor() : super()

    constructor(delegatesManager: AdapterDelegatesManager<T>) : super(delegatesManager)

    constructor(vararg delegates: AdapterDelegate<T>) : super(*delegates)

    // Java to Kotlin bridge for null-safety
    @Suppress("RedundantOverride")
    override fun getItems(): T? {
        return super.getItems()
    }
}

Apart from being an ugly workaround, it also will only work for accessing items outside of the adapter. In a sub-class, it will still default to the protected T items variable without null-safety.

Is there anything that can be done about that? Or am I missing an obvious solution to that problem? Thank you!

Sab44 commented 2 years ago

I have created this PR, simply adding @Nullable annotations: https://github.com/sockeqwe/AdapterDelegates/pull/107

sockeqwe commented 2 years ago

thanks for your contribution. I will take a look at it tomorrow.

sockeqwe commented 2 years ago

just out of curiosity: what is your use case to actually access ˋitemsˋ or create a subclass of ˋListDelegationAdapterˋ

Sab44 commented 2 years ago

Sure, I will try to explain. Our entire app is written in Kotlin. We have a very dynamic screen (consisting of a recycler view mainly) where depending on some conditions we might need to insert or remove something into our adapter's list. For this we have declared our own adapter, subclassing ListDelegationAdapter and passing our various AdapterDelegates inside the constructor. In the body of that adapter we have functions like e.g.

    fun insertItem(index: Int, item: MyDisplayItem) {
        items = items?.toMutableList()?.apply { add(index, item) }
        notifyItemInserted(index)
    }
    fun removeItem(index: Int) {
        items = items?.toMutableList()?.apply { removeAt(index) }
        notifyItemRemoved(index)
    }

where items is not considered nullable therefore no issues arise if we omit the ? after a call to it.

Additionally, some of the views we have are a bit complex with nested scrolling and we have some custom tracking inside our fragment, where we need to get the position of certain views, something like:

    newsPromotionAdapter.items?.indexOfFirst { it is MySpecificDisplayItem }

The second case can be worked around with that custom base adapter, but the first case (inside our adapter) will always allow a non-nulllable access to the items variable.

This is not something that prevents us from using the library of course. It is just a safety measure we would like to have for future implementations and for using this excellent library throughout more places in our app.

Thank you for looking into it.

sockeqwe commented 2 years ago

Thanks for sharing

Sab44 commented 2 years ago

Thank you for reviewing and merging! I will close this issue.

Sab44 commented 2 years ago

@sockeqwe any idea when we will have the change available in a release? Thanks!

sockeqwe commented 2 years ago

@Sab44 could you give the latest snapshot a try to see if everything works as expected? If that is the case I will create a new release

Sab44 commented 2 years ago

@sockeqwe can confirm with the snapshot version that for both the inherited items variable inside the adapter as welll as calls to getItems() outside the adapter, the Kotlin compiler will enforce null-safety.

Sab44 commented 2 years ago

@sockeqwe any news on a release? Thanks.

sockeqwe commented 2 years ago

Sorry, I almost forgot about it. I'm on vacation right now, I'm back on Wednesday. Will then create a new release!

sockeqwe commented 2 years ago

it is released now as 4.3.2

Thanks again for your contribution