Making use of Collection instead of AdapteeCollection
Lukard opened this issue ยท 7 comments
Hi everyone!
I would like to know if we could change RVRendererAdapter
and RVListRendererAdapter
collection
type to Collection<T>
instead of AdapteeCollection<T>
.
It causes me trouble in the following situation. I would like to update just one item from the list instead of performing a full diffUpdate
. Since there is no set
method in AdapteeCollection
I can only do the following:
List<T> list = (List<T>) collection;
list.set(position, newElement);
notifyItemChanged(position);
The drawback of this approach is that it is performing a cast over collection
assuming that the implementation inherits from List.
Another way of tackling this issue would be copying all elements into aList<T>
and performing the set
operation there. But that is even worse given the original collection is already a List<T>
and we would be doing a useless copy.
Are there any arguments against using Collection
instead of AdapteeCollection
?
@pedrovgs If you agree with the solution I can open a PR to propose the changes so you can take a look!
@Lukard "instead" may not be the best solution if we want to avoid a breaking change. However, I guess if we would like to implement this change we could make the adapter to receive either an AdapteeCollection
or just a Collection
. However, why don't you use ListAdapteeCollection
which already extends from ArrayList
?
@pedrovgs I use ListAdapteeCollection
but RvRendererAdapter
and RVListRendererAdapter
are holding an instance of AdapteeCollection
(instead of ListAdapteeCollection
). So if I want to add/extend those adapters with new functionality that manipulates the collection
I need to cast it to List<T>
or something like that, since AdapteeCollection
provide much less functionality than Collection
.
Cast it to List
may be dangerous because someone may pass an instance to the adapters that is different than ArrayList
or any other non-Collection
types.
I know it may be a breaking change, but since the library hasn't been updated for a long time I would say it is pretty stable and a breaking change shouldn't be a big deal.
Would like to see a PR with requested changes so we can discuss about way to do it?
It's been a long time since I don't use this library, I'm using compose now...However, when I used it I always used RVRendererAdapter
API and I wanted to change the class to extend from collection. I think we can can just avoid the breaking change by making the adapter extend from collection and receive a collection in construction too. If you don't mind, could you please change the RendererAdapter
and RVRendererAdapter
to implement Collection?. You can mark AdapteeCollection as deprecated as well ๐
Thank you @pedrovgs I'll do that
Offtopic: BTW lucky you that could start using Jetpack compose. I want it to introduce it at New Work, but since we are working in a really big app we need it to be in an stable state. Unfortunately it is still in alpha. Hope it reaches to a stable release soon.
Hi @pedrovgs!
It has been a lot of time since my last message, but priorities changed at work and I couldn't tackle the issue until now.
I need to use List
instead of Collection
to make it work, all details are described in the PR. Let me know if you have any doubts.
Here you have the PR: #58
Thanks a lot ๐
Releasing a new version right now.