Passing listener to customized extended View in Android

Question

I extended NumberPicker with several methods needed in my application. When I came to defining a OnValueChangeListener this is how I did it:

import android.widget.NumberPicker;

interface ValueChangeCallback {
    public void execute(RemListView list, int oldVal, int newVal);
}

public class RemListView extends NumberPicker  {
    ValueChangeCallback mValueChangeCallback;
    RemListView thisList = this;

    public void setValueChangeCallback(ValueChangeCallback callback) {
        mValueChangeCallback = callback;

        setOnValueChangedListener(new NumberPicker.OnValueChangeListener() { 
            @Override
            public void onValueChange(NumberPicker picker, int oldVal, int newVal) {
                mValueChangeCallback.execute(thisList, oldVal, newVal);
            }
        });
    }
}

This code works fine but I'm new to Android and somehow I'm not sure I did it the proper way so I'm asking for a code review.

Do I have to save the callback in member variable?

I couldn't access this from within callback so I introduced thisList variable. Can it be avoided?

Answer 1

You don't need either thisList or the mValueChangeCallback in your class.

As your value change listener is an anonymous inner class, it has access to the outside this object, which you can access with RemListView.this.

As your callback is passed as a parameter and you want to use it inside the anonymous inner class inside the method that the parameter is passed to, you can mark it as final to make it usable.

public void setValueChangeCallback(final ValueChangeCallback callback) {
    setOnValueChangedListener(new NumberPicker.OnValueChangeListener() { 
        @Override
        public void onValueChange(NumberPicker picker, int oldVal, int newVal) {
            callback.execute(RemListView.this, oldVal, newVal);
        }
    });
}

I do believe though that since you're not modifying anything about the listener functionality, I think you don't need this method at all. The NumberPicker.OnValueChangeListener is already publically available because you extend the NumberPicker, you can't hide it. Therefore, the ValueChangeCallback just sounds useless to me. I would recommend using the NumberPicker.OnValueChangeListener instead.

Answer 2

I think you have missed the point with the NumberPicker here. There is no need to add in a new level of abstraction for your application.

Right now your 'calling' code needs to implement a ValueChangeCallback class, and call setValueChangeCallback(callback). The ValueChangeCallback needs to implement the execute() method.

Sure, this works, but, the design is supposed to be a whole lot simpler, just do:

Make your calling code implement a NumberPicker.OnValueChangeListener and the onValueChange() method. Then, it can call setOnValueChangedListener directly.

Basically, your code should be simplified to:

import android.widget.NumberPicker;

public class RemListView extends NumberPicker  {

}

and your calling class should be changed to have a NumberPicker.OnValueChangeListener instance instead of a ValueChangeCallback instance, and, well, that's it.

Oh, and since your RemListView class is now empty, unless you have other stuff in there, you may as well delete the class and use NumberPicker directly.