Android onClickListener implementation best practices

Question

There are four ways to add an onClickListener to a clickable View (button, for example):

set the onClick attribute in the layout file which points to a method in the activity,
create an anonymous inner class,
assign the onClickListener to a private member variable.
have the Activity context implement the onClickListener interface.

So my question is, how do you choose one of these implementation techniques over another? Is there a best practices according to certain conditions, or is it just a matter of programmer preference?

Anonymous inner classes are dangerous and can lead to leaking the outer class, so I would avoid this pattern. I would suggest using either a static inner class with a WeakReference to the outer class if a reference is needed, or having the Activity itself be the onClickListener. — Michael Krause, May 03 '16 at 21:03
Avoid the member variable option as well? If so, why? Thanks! — JaeW, May 04 '16 at 01:44
In the case of a member variable holding a reference to an onClickListener, you're still going to need a declared class to implement the onClick method. So, you'll again need to make sure it is static if it is declared inside the scope of your outer class, and that if it needs a reference to your outer class (whether it is declared as an inner class or as a separate class declared elseshwere) that it is a WeakReference. — Michael Krause, May 04 '16 at 03:19
@Michael Krause can you show memory leak example that doesn't use AsyncTask, Handler or any threading? — Maxim G, May 04 '16 at 17:13
Simply declare a static variable in one of your classes and have it hold onto an Activity instance without setting it to null afterwards. Static variables live on the heap separate from a class instance. Since the garbage collector works by finding every object that is not reachable, this static variable references will never get garbage collected. Fire up Android Device Manager and go spelunking and you'll probably be surprised at what things aren't being garbage collected. — Michael Krause, May 04 '16 at 19:17

score 4 · Accepted Answer · edited May 23 '17 at 11:59

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.

So in Android, the interface has already been created in the form of onClickListener(). If an Android Button was implemented (as opposed to your code above), there is no (existing) constructor which accepts the onClickListener() as a param; instead you would have to use setOnClickListener() which Button inherits from the View class. Is this all correct? — JaeW, May 27 '16 at 03:03
Example only shows the idea of the callback pattern. In Android api you will use set method like `setOnClickListener() ` which expects interface [View.OnClickListener](https://developer.android.com/reference/android/view/View.OnClickListener.html). BTW same concept is recommended for chatting between a Fragment and the host Activity ([example](https://developer.android.com/training/basics/fragments/communicating.html)). — Maxim G, May 27 '16 at 04:58

score 2 · Answer 2 · answered May 03 '16 at 21:00

I never use the onClick attribute as it ties the layout to a specific Activity (it must find the method via reflection). It doesn't work on Fragments.

Options 2 and 3 are virtually identical. Option 3 might be more advantageous if you want to use the private member as the OnClickListener of multiple views.

Option 4 is close to option 3. One key difference is that it changes the class declaration, so if it's important to you to keep the class declaration free of interface implementations (or perhaps you need to maintain binary compatibility of some kind), you may not want to use this option.

My advice is to avoid option 1 and choose whichever fits your code style best. You also aren't required to use the same approach in every single place in your code.

Thank you. Very helpful. Additional question: Why would you want to keep a class declaration free of interface implementations? — JaeW, May 04 '16 at 00:27
@JaeW One example is you may want to maintain binary compatibility with a previous version of that class (perhaps if this were in an SDK of some sort). Also, if you make the class implement the interface, anyone who can somehow get an instance of it can use it wherever that interface is expected, and you might not want that to be possible. — Karakuri, May 04 '16 at 06:45

score 2 · Answer 3 · answered Sep 18 '18 at 05:53

There is a four ways to use OnClickListener.

First way

To define OnClickListener within the method call site.

public class MainActivity extends AppCompatActivity {

    @Override
    protected void onCreate(@Nullable Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        Button button = findViewById(R.id.myButton);
        button.setOnClickListener(new OnClickListener() {
            @Override
            public void onClick(View v) {
                // do something
            }
        });
    }
}

First reasons why to avoid this is because it clutters up onCreate method. This becomes even more apparent when you want to observe click events from multiple views.
The next reason to avoid this is because it doesn't promote code reuse if several buttons should do the same.

Second way

The second way is almost the same as first except implementation to field is assigned in the class.

public class MainActivity extends AppCompatActivity {

    private View.OnClickListener clickListener = new View.OnClickListener() {
        @Override
        public void onClick(View v) {
            // do something
        }
    };

    @Override
    protected void onCreate(@Nullable Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        Button button = findViewById(R.id.myButton);
        button.setOnClickListener(clickListener);
    }
}

This way is pretty the same as first one, only advantage is that the method could be reused for several buttons.

Third way

This way is to declare an inner class to implement OnClickListener. If it will be used multiple times is better to define the instance as a field.

public class MainActivity extends AppCompatActivity {

    @Override
    protected void onCreate(@Nullable Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        Button button = findViewById(R.id.myButton);
        button.setOnClickListener(new ButtonClick());
    }

    class ButtonClick implements View.OnClickListener {
        @Override
        public void onClick(View v) {
            // do something
        }
    }
}

The advantage of this way is that it helps to organize the code. You can easily collapse this internal class and forget about it until you need to look at it.
The other good reason is that it could be turned in public class and reused in other app areas.

Fourth way

The fourth way involves Activity to implement OnClickListener.

public class MainActivity extends AppCompatActivity implements View.OnClickListener {

    @Override
    protected void onCreate(@Nullable Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        Button button = findViewById(R.id.myButton);
        button.setOnClickListener(this);
    }

    @Override
    public void onClick(View v) {
        // do something
    }
}

The first disadvantage that this way creates a public method in the Activity and there you should pass this activity when setOnClickListener is called.
Second reason to avoid this way is that if another button is added you should determine which button was clicked. Then you should use switch() or if() statements. It isn't performed because it wastes a cycle or several for each button click.
Last disadvantage for this way is that difficult to organize a class. In example you have an Activity which implements multiple interfaces. Suddenly all of the methods from these interfaces are intertwined together, this becomes more evident after you’ve added methods to some of those interfaces. Also now you can't add an interface with method named onClick.

There is some differences between these ways, but you should choose your way according your code and needs

Android onClickListener implementation best practices

3 Answers3

First way

Second way

Third way

Fourth way