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.

• jackhab: Can you elaborate on why do I need to make the callback final?
• Simon Forsberg: @jackhab Because otherwise Java cannot be certain that the callback parameter is to the same object reference. final tells Java that the value (reference) cannot be modified, so it is safe for anonymous inner classes to use it at a later time.

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.

• Simon Forsberg: I totally agree! This RemListView class is absolutely useless.
• jackhab: Well, guys, of course I extended the class not only for the sake of changing the name from NumberPicker to RemListView. It does have other members and methods specific to my app (e.g. it allows selecting an element by text value instead of index etc.) I new I could implement NumberPicker.OnValueChangeListener in main activity and pass it directly I just thought if my application works with RemListView class it shouldn't be aware if class's internals. Moreover, the callback applies changes to RemListView members so it cannot be aware only of NumberPicker.
• Simon Forsberg: @jackhab From what you explain now your code sounds completely different. It would have been a good idea to mention this right from the start (now it is too late as by adding that information you would invalidate this answer). You may post a follow-up question if you would like about your real code. I have written an answer for your specific question though.