Best Practice for Defining Button Events in Android

Best practice for defining button events in android

Why not registering onClick event in the XML layout and then handle it in the code. This is how I would do it:

<Button
android:id="@+id/my_btn"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:text="Click me"
android:onClick="onBtnClicked">
</Button>

and now create a method that would handle clicks

public void onBtnClicked(View v){
if(v.getId() == R.id.my_btn){
//handle the click here
}
}

Alternatively, you can set the OnClickListener individually for each item in the code. Then use the if/else or switch statements to determine the origin.

This way you can have one method that handles all buttons from one layout.

UPDATE:

Although this is a valid approach I would strongly recommend the second option. It's cleaner and easier to maintain especially when you work with fragments.

Android onClickListener implementation best practices

Here we use so called callback pattern.

public class Button {
private Callback callback;

public Button(Callback callback) {
this.callback = callback;
}

public void update() {
// Check if clicked..
callback.onClick(this);
}

public interface Callback {
public void onClick(Button Button);
}
}

Button b = new Button(new Callback() {
@Override
public void onClick(Button b) {
System.out.println("Clicked");
}
});

In our case onClick handler implements the interface View.OnClickListener.

Key points:

  • consistency with activity/fragment;
  • access to the members of activity/fragment;
  • readability;
  • @Michael Krause showed one more good point about memory leaks;

1) Attribute in the XML file can be used only for activity, as @Karakuri mentioned it uses reflection which is slow.

2) Anonymous inner class has special rules for access to the members of enclosing class (check [1], [2]). There are some situations when memory leaks can happen (ex. threading with AsyncTask, Handlers).

3) Here you have a full access to the members of enclosing class.

4) Is a variation of 3d.

Readability depends on your handler size, small logic can be ok to inline, but for larger blocks of code consider 3d and 4th.

How to handle card's button OnClick event in GridView? Best practice

Looks like nobody knows how to do this. So I found solution myself with help of @Dimitar G. and @Konstantin Kiriushyn. Thank you, guys.

1) I will create my own custom CardView using Compound View system, which will be pretty simple: LinearLayout + ImageView + TextView + Button.

public class TopicCardView extends LinearLayout {

private ImageView mImage;
private Button mButtonMenu;
private TextView mTitle;

public TopicCardView (Context context) {
initializeViews(context);
}

private void initializeViews(Context context) {
LayoutInflater inflater = (LayoutInflater) context .getSystemService(Context.LAYOUT_INFLATER_SERVICE);
inflater.inflate(R.layout.topic_card_view, this);
}

private void setTitle(...) {
...
}

private void setImage(...) {
...
}

private void setMenuClickListener(...) {
...
}

// and so on...
}

2) Then I will create method called createListOfGridCardsFromDB(...) in Activity\Fragment. It will generate list (LinkedList) of my custom CardViews (and it will also set titles\images and listeners to CardViews).

3) And then I will pass this generated LinkedList of my CardViews to GridViewAdapter.

This system makes able to use only one Adapter for all my card-grids in app. It also makes able to do nothing with clicks, interfaces, listeners and stuff in Adapter.

Kotlin onClick events and architecture best practices

also I'm not sure I like this unnecessary ViewModel-Fragment ping-pong

That's a normal reaction that everyone has to it. The benefits are decoupling of View from ViewModel, which makes the View be just a dumb class that handles user input and display results to the user, while the ViewModel is the smart class that handles the logic. This makes the logic easy to unit-test, as you don't need the entire application to be running (which a Fragment would require). Views are quite boiler-place heavy, so it's great to keep the logic in another place, where its easier to read as it isn't surrounded by a ton of view code.

My events need a context [...]

The events are just signals to the View to perform a specific action. You will not be dealing with Context in the ViewModel, instead, the View can do what it needs with its Context, once it receives the signal to do so.

Events can contain data, but not Android-specific data. If you want to for example pass a Drawable (or any other resource) through an event, you would pass only its resource ID, which the View can resolve into a Drawable using its Context.

E.g. the ViewModel emits a ChangePhotoEvent,

The whole flow of a user clicking, and it resulting in a Dialog being shown, would look like this.

The View handles the click, and tells the ViewModel about it:

android:onClick="@{() -> profileViewModel.onChangePhoto()}"

The ViewModel now determines what should happen, it can ask Repositories for data, check some conditions, etc. In this case, it just wants to display a photo picker to the user, so it sends an event to the View, with just enough information for it to know what is being asked of it:

The way you had OneTimeEvent implemented is worse than it has to be. Let me simplify it for you:

public interface OneShotEvent

abstract class BaseViewModel : ViewModel() {
private val _events = MutableLiveData<OneShotEvent>()
val events: LiveData<OneShotEvent> = _events

fun postEvent(event: OneShotEvent) {
_events.postValue(event)
}
}
class MyViewModel: BaseViewModel() {

data class ChangePhotoEvent(val updatePhotoUrl: String) : OneShotEvent

// for events without parameters, define them as objects
// object ParameterlessEvent : OneShotEvent

fun onChangePhoto() {
postEvent(ChangePhotoEvent(Authentication::updatePhotoUrl))
}
}
class MyFragment : Fragment() {

override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)

viewModel.events.observe(this) { event ->
when (event) {
is ChangePhotoEvent -> {
// display the dialog, here you can use Context to do so
showImagePicker(event.updatePhotoUrl)
}
// omit 'is' if event is an object
// ParameterlessEvent -> {}
}
}
}
}

I didn't understand what exactly you were doing with Authentication::updatePhotoUrl. In onActivityResult, you should call the ViewModel again, with the result you received: viewModel.onPhotoChanged(data?.data), and the ViewModel should call Authentication.updatePhotoUrl(). All logic happens in the ViewModel, the View is just a relay of user-events.

If you need to retrieve any data from an API, the ViewModel has to do that, on a background thread, preferably using Coroutines. Then you can pass the data as a param of the Event.


You could check out a framework, like RainbowCake, which provides base classes and helpers for this kind of stuff. I've been using it for a while, you can see a full project where I'm using it here.

best implementation of onclick()

In that case, I might add implements View.OnClickListener to the activity class, and then make a single onClick that looks like:

@Override
public void onClick(View v){
switch(v.getId()){
case R.id.button1:
// handle button1 click
break;
case R.id.button2:
// handle button3 click
break;
case R.id.button3:
// handle button3 click
break;
// etc...
}
}

This only really makes sense if the actions of all those buttons are somehow related, and if they are then it can significantly reduce code duplication. You would still have to do view.setOnClickListener(this) for each one, too.

Alternatively, if you want you can remove the implements View.OnClickListener and the @Override and simply register your method in the XML, like Kevin said. That would allow you to have multiple of these "grouped" click listeners.



Related Topics



Leave a reply



Submit