views:

69

answers:

6

I would like to ask what is the good practice on using anonymous classes vs. named inner classes?

I am writing an Android application, which includes many UI elements (buttons, text fields, etc). For many of them I need some kind of listeners, so in onCreate of the application I have bunch of quite small anonymous classes like:

someButton.setOnClickListener(
    new View.OnClickListener() {
        public void onClick(View v) {
            // do something...
        }
    }
);

Each of of such anonymous class is 5 - 20 lines big - small enough and fits good for recommendations from Java™ in a Nutshell book:

In general, you should consider using an anonymous class instead of a local class if:

  • The class has a very short body.
  • Only one instance of the class is needed.
  • The class is used right after it is defined.
  • The name of the class does not make your code any easier to understand.

But the problem, IMO, is that onCreate becomes quite big and the code becomes more complex to read and understand by quick looking in to it. It is still easy to understand, but simply too big.

So what would be better practice in such case - have bunch of small inner sub-classes, where each of it is nicely separated , but used just once or better keep using anonymous classes instead?

A: 

As the quote suggests, this is something that is subjective. You need to figure out what counts as "very short" and "easier to understand" for yourself, to determine at what point the code size crosses the line from something that makes sense as an anonymous class to something that makes sense to be it's own unit.

Any answers anyone would give you on this is based on their own subjective measures of what "short" is and how many lines of code it takes to no longer be "short".

matt b
+2  A: 

Refactor the onCreate into separate methods grouped by common functionality so you have logical units. If the GUI is complex then it will help you later.

Thorbjørn Ravn Andersen
+2  A: 

I don't think there is a clear answer one way or the other. Both styles work fine, its really just what you prefer.

Another option is to have

the contents of each onClick by a single function call, which will keep the anonymous classes very short. I.e:

someButton.setOnClickListener(
    new View.OnClickListener() {
        public void onClick(View v) {
            doSomeButtonClick();
        }
    }
);


private void doSomeButtonClick() {
  // do something
}
Mayra
+1  A: 

I don't know if it's considered by the community as a best practice, but when this sort of class gets too big, I create a package listener and create my class in it. Why use inner class or anonymous class if it gets in your way later in your code.

But most of the time, if this kind of class gets big, it's because you didn't delegate enough. Maybe other classes should have methods that help your listener to be lighter.

Colin Hebert
+1  A: 

I do Desktop/Swing applications, but I think the concepts are the same.

I prefer to handle all of the events in one class, so I create a Controller class and implement all of the listeners I need in that one class. Then, I create a method on the panel for each type of listener. It adds the listener to each component in the panel that it needs to listen to. I pass the panel as the first argument to the Controller class's constructor and have it call each of those add listener methods on the panel. Then I instantiate the controller when I instantiate the panel.

This gives me several advantages:

  • All of the event handling code is in one class.
  • Any crossover in functionality between events is easily caught and handled.
  • Any state that's affected can be stored and controlled in one class.
  • All event handling code is separate from the component layout code.
  • The View layer (Panel) doesn't know anything about the Controller layer.

That being said, if the events you're hooking up all do simple things, and these things don't conflict with other events, then it's not wrong to use anonymous classes.

Erick Robertson
This is great technique you're using. I think, it is very useful when you have many interactions between UI components. Although Mayra suggestion probably fits more for my needs, but still - really good idea. I'll keep it in my mind for future, thanks!
Laimoncijus
I agree with you.
Erick Robertson
A: 

You have several advantages to putting these items in named classes.

The first is the one you mention--it will make the onCreate() method more concise and understandable.

The second is that the name should quickly identify which logic goes with which button, which will make the code cleared

The third is easier separation of duties. If you should decide to go to an IoC model, you'll have a much easier time injecting named implementations of the listeners.

Matthew Flynn