0

My boss says I write Listeners wrong, and can not bring any good arguments as the why. The way I usually do it is this:

View.OnClickListener onClickListener = new View.OnClickListener() {
        @Override
        public void onClick(View view) {
            if (R.id.basketCancel == view.getId()) {
                Observer.getInstance().send(MessageType.SCAN_STATUS, new ScanEvent(true));
                dismiss();
                return;
            }
        }
    };

and

 cancel.setOnClickListener(onClickListener );

He requires me to write it like so:

private class ClickButtonHandler implements View.OnClickListener {
    @Override
    public void onClick(View v) {
        if (R.id.basketCancel == v.getId()) {
            Observer.getInstance().send(MessageType.SCAN_STATUS, new ScanEvent(true));
            dismiss();
            return;
        }
    }
}

and

private ClickButtonHandler clickButtonHandler = new ClickButtonHandler();
...
 cancel.setOnClickListener(clickButtonHandler);

I believe that it complicates things. He believes that so I can implement multiple listeners everywhere to use one object of this class. But I usually use only View.OnClickListener And the structure itself is not like me. How do I explain to him and to prove that I do better? or do you think he's right? I can not understand?

CodeMonkey
  • 11,196
  • 30
  • 112
  • 203
  • I can't read your boss mind :p may be he want to achieve reusability and easy maintenance. Also java object is not free, may be he wants good application performance. etc etc etc. – Muneeb Nasir May 01 '15 at 13:46
  • @Pavel please consider clarifying provided answers (via comments) and accepting them if helpful. – kiruwka May 04 '15 at 18:38

3 Answers3

1

This question is hard to answer correctly, because it is opinion-based in my point of view. So I'll focus on the pros and cons of both approaches:

In case you need multiple instances of one and the same implementation, a named class makes sense. You can easily identify those spots by searching for duplicate listener code. Multiple instances are usually only required, if your listener class holds any state, i.e. has one or more fields. (If it is stateless (no fields), it is sufficient to have one single instance only).

But in case you need a certain listener implementation just once, or you can reuse the same instance of a certain listener implementation, your approach (anonymous inner class) seems to be more suitable to me as it simplifies your code.

isnot2bad
  • 24,105
  • 2
  • 29
  • 50
1

Short answer is : the approach to use depends on your application, i.e. you can start with "quick" anonymous inner listeners and end up refactoring them in named classes (or better have your class implement interfaces directly) as your code base evolves.

Long answer:

From java point of you, you are doing essentially the same thing: instantiating anonymous inner class (first approach) is equivalent to declaring an inner named class (second approach) and then instantiating it.

The only difference is whether this instance is a local variable (first approach) or kept as outer class field to be reused on multiple occasions (second approach).

First approach usually allows for "faster" coding, i.e. instantiate subclass without writing full declaration.
As a benefit of first approach you can read local [final] variables and parameters of the enclosing outer method/block inside your anonymous listener, i.e.:

    void outerClassMethod(final URL url, final String param, ... etc) {
            final int outerInt = updateMaxCount(...);

            // you have access to url, param, outerInt in your anonymous instance :
            View.OnClickListener onClickListener = new View.OnClickListener() {
                @Override
                public void onClick(View view) {
                    if (url.getHost().equals(...)) {// <- access url param here 
                        int local = outerInt * 2; //<- access outer method's outerInt
                    }

                }
            };
    }

However if you find your multiple anonymous listeners using same code you should consider putting common code in named class and reusing it (second approach).

Edit : Regarding second approach, please Note that situation may require to pass different listener objects, even if you declare inner class ClickButtonHandler, i.e. you might need :

button1.setOnClickListener(new ClickButtonHandler());
...
//  and somewhere else
button2.setOnClickListener(new ClickButtonHandler());

Otherwise, instead of inner class, I would rather have main class implement OnClickListener directly, i.e. :

public class MyOuterClass implements OnClickListener, SomeOtherInterfaces {

     // and then pass itself as listener:
     cancel.setOnClickListener(this);

}

Hope that helps.

kiruwka
  • 9,250
  • 4
  • 30
  • 41
  • The "more flexible" is iffy. That anonymous inner class may be able to access fields and parameters of the enclosing outer method, but you can't use it with anything outside that method without passing around references. And if you instead make that enclosing outer method a member of the inner named class, then you get all those benefits you mentioned plus the ability to reuse the class (though the downside is now the class may have methods it doesn't always need). I think which is the better approach is application dependent. – iheanyi May 01 '15 at 14:35
  • @iheanyi yes, good point. It is *very much* application dependent, I edited my answer, let me know if you disagree – kiruwka May 01 '15 at 14:49
  • I do indeed. Thanks for responding. – iheanyi May 01 '15 at 14:52
0

You can have a view with multiples buttons or clickable areas. In this case it would be lighter to use only one OnClickListener and perform a switch on the view's id

myOnClickListener = new OnClickListener(){
    public void onClick(View view){
        if(view.getId() == view1.getId()){
            ...
        }else if(view.getId() == view2.getId()){
            ...
        }
    }
}
view1.setOnClickListener(myOnClickListener);
view2.setOnClickListener(myOnClickListener);