Allow more type specific reactions without switch statements #6
Comments
|
Hello @dalewking, by "complex interactions" you mean handling different click listeners for different elements within the same item type? If so, as you said, you can do that using the OnBindListener but, again as you said, there is only one for all the item types so the code wouldn't be so clean. However, there's another way you can achieve that using this library and it is handling the clicks in the layout itself. I think that way will fit your needs. Here's an example: <layout ...>
<data>
<variable name="item" type="...Item" />
</data>
<LinearLayout ...>
<ImageView ...
android:onClick="@{v -> item.onItemImageClick(v)}" />
<TextView ...
android:onClick="@{v -> item.onItemTitleClick(v)}" />
<TextView ...
android:onClick="@{v -> item.onItemDescriptionClick(v)}" />
</LinearLayout>
</layout>public class Item {
...
public void onItemImageClick(View view) {
// handle click on image
}
public void onItemTitleClick(View view) {
// handle click on title
}
public void onItemDescriptionClick(View view) {
// handle click on description
}
}
Can you provide more information about that? |
|
Please, let me know if that worked for you. |
|
Well, yes and mostly no. Sure, of course everything works if you can do everything through calls on the item itself, but sometimes the item is just a dumb model object that doesn't have the ability to handle the extra functionality. Perhaps it is cleaner if the list doesn't contain a bunch of dumb model objects but instead is a list of item view models that wrap the dumb model objects, but it would be nice to not force that on to the user. In my case I have items that have enable/disable switches on them and the ability to delete them (along with other items that are just headers and have no functionality). The items themselves are just dumb data objects and you have to pass them to other calls to handle those operations. If there was a way to create your own type specific ViewHolder classes that would be much more powerful. The thought on unbind was some way to notify the user that onViewRecycled was called on the holder so they can release resources. |
|
I will consider adding this kind of functionality for next versions. Actually, I thought about it before but decided not to include it as the intention was to use Data Binding, and listeners (onBind/onClick...) are there for cases where you can't use Data Binding or just want to declare the functionality in a quick and explicit way. However, I don't think that creating the specific ViewHolders is the way to do it, at least for this library, as the point here is actually avoiding them... More "customizable" listeners for specific item types could be an option, though. Meanwhile, I want to let you know that you can still keep your dumb models and handle the clicks in other classes: <layout ...>
<data>
<variable name="item" type="...Item" />
<import type="...Listeners" />
</data>
<LinearLayout ...>
<ImageView ...
android:onClick="@{v -> Listeners.onItemImageClick(item, v)}" />
<TextView ...
android:onClick="@{v -> Listeners.onItemTitleClick(item, v)}" />
<TextView ...
android:onClick="@{v -> Listeners.onItemDescriptionClick(item, v)}" />
</LinearLayout>
</layout>public class Listeners {
public static void onItemImageClick(Item item, View view) {
// handle click on image
}
public static void onItemTitleClick(Item item, View view) {
// handle click on title
}
public static void onItemDescriptionClick(Item item, View view) {
// handle click on description
}
} |
|
I know it is pretty much the same, but now you are keeping your models classes as simple as possible and can handle the events anywhere else, maybe even as an internal class in the Activity/Fragment where you are using the adapter. |
|
I tried playing with it a bit and here is what I came up with moving modified OnBindListener to map call and adding OnUnbindListener as a patch file: |
|
After posting that, the unbind should probably also pass the view. It seems with this also that the click listeners are not really necessary as you can just use the bind listener and set them yourself if you wanted them. |
|
I'm aware you do not make pull requests on GitHub but, honestly, I don't know how to apply a patch file. Could you copy/paste the whole file here or somewhere else so I can take a look at it more easily? |
|
Wasn't even that. I recently did a PR for RxJava, just didn't have time to On Sun, Oct 9, 2016 at 1:46 AM, Miguel Ángel Moreno <
Dale King |
|
Well... It looks fine but my main concern is what I called the LayoutHandler (the capacity to choose different layouts depending on the position of the item in the adapter or any internal status of the item itself). Setting the OnBindListener, for example, while using the LayoutHandler doesn't seem to be supported with the modifications you made. Sure it would be possible to do but I can't see it happening without adding extra complexity than just the method returning the layout id. So we would be adding an additional way of doing something that can already be done at the expense of the LayoutHandler, which I find it quite useful. In my opinion, the The solution could be passing also the specific binding class for each type to use it for the generics thing but, again, it would kind of "ruin" the simplicity of the current LayoutHandler (its method should then return the layout id AND the binding class for this layout). A good combination of all those possibilities passing the specific binding class or using the default ViewDataBinding when not useful/needed could be great, but it should be very well thought. I'd like to know your opinion. |
|
Hello @dalewking I've been working on a solution similar to what you posted. I've done some modifications so the item and its binding are both cast to their appropriate classes in the listeners, except for the OnUnbindListener. The Can you tell me some good cases where having this OnUnbindListener would be useful? What kind of resources should be released there? |
|
In the code I was looking at converting, I have type specific view holder classes for the various types. There are 2 variables bound to the layout, the item and the holder itself. The holder is used as the target of various event methods. In reality, the unbind handling just does: So ViewDataBinding is enough in my case. This list in particular maintains a list of timers and alarms. Some of them have a text view that counts down the time so the text keeps updating. Setting the holder to null will cause the count down text view to quit counting down. |
I actually, I double checked and I was wrong about that. My unbind is just setting the holder variable to null on the layouts which unregisters the listeners from the views is all the unbind is really doing. |
|
LastAdapter 2.0 will include specific listeners for each type and the Please, see #10 |
In my own use of data binding and view holders I have some more complex interactions from the view to the view holder. I have several controls within an item that have their own listeners. So I have different ViewHolder types and I actually have a variable in the layout for the holder and I can bind to or use methods on the ViewHolder to do listeners or provide additional data to the view.
Looking at this library as a possible alternative to my complex Adapter and ViewHolder, I don't see a good way to handle these more complex interactions. It is possible using the OnBindListener, but that is a single listener for all types. You end up having to do a switch statement and probably a cast. That is a code smell and what we would like to avoid by using a library like this.
I am trying to think what is the best way to accomplish this in a minimally invasive way. What I am envisioning is a version of the map call that also accepts a parameter of a class that extends LastAdapter.ViewHolder which has an onBind method that subclasses can override to do custom work (and FYI you should add support for unbinding as well to clean up things when a view is removed from the list).
Just sort of thinking out loud here.
The text was updated successfully, but these errors were encountered: