java events and listeners, bad implementation?

I am currently implementing custom events and listeners according to the code posted below. I have been told that this is a very dirty implementation and that this needs to be changed. However, i am very new to java and android and do not see what is wrong with the current implementation. The way i have it below works and seems to be doing everything i needed it too. I was wondering if some people could please take a look at my code and make some suggestions on what i should change and what i am doing wrong. Taking my example and modifying it so that i can see what your talking about would be greatly appreciated.

Thanks in advance!

/* SmartApp.java */
public class SmartApp extends Activity 
{
    private ConnectDevice cD = new ConnectDevice();
    private DataRobot dR = new DataRobot();
    private DataBuilder dB = new DataBuilder();
    private DataSender dS = new DataSender();
    public void onCreate(Bundle savedInstanceState) {
       super.onCreate(savedInstanceState);
       setContentView(R.layout.intro);

    cD.addDataReceivedListener(new DataReceivedListener() {
        @Override
        public void dataReceivedReceived(DataReceivedEvent event) {
            // TODO Auto-generated method stub
            dR.analyzeData(event.getData());
        }
    });
    dR.addDataAnalyzedListener(new DataAnalyzedListener() {
        @Override
        public void dataAnalyzedReceived(DataAnalyzedEvent event) {
            // TODO Auto-generated method stub
            dB.submitData(event.getData());
        }
    });
    dB.addDataBuilderListener(new DataBuilderListener() {
        @Override
        public void dataBuilderReceived(DataBuilderEvent event) {
            // TODO Auto-generated method stub
            dS.sendData(event.getData());
        }
    });
      }
}  

---

/* ConnectDevice.java
 * This class is implementing runnable because i have a thread running that is checking
 * the contents of a socket. Irrelevant to events. */
public class ConnectDevice implements Runnable {

    private List _listeners = new ArrayList();
    private String data;

    /* Constructor */
    public ConnectDevice() {// does some socket stuff here, irrelevant to the events}
    public void run() {// does some socket stuff here, irrelevant to the events}

    public synchronized void addDataReceivedListener(DataReceivedListener listener) {
        _listeners.add(listener);
    }
    public synchronized void removeDataReceivedListener(DataReceivedListener listener) {
        _listeners.remove(listener);
    }
    private synchronized void fireDataReceivedEvent(String temp) {
        DataReceivedEvent dRE = new DataReceivedEvent(this, temp);
        Iterator listeners = _listeners.iterator();
        while(listeners.hasNext()) {
            ((DataReceivedListener)listeners.next()).dataReceivedReceived(dRE);
        }
    }
    public interface DataReceivedListener {
        public void dataReceivedReceived(DataReceivedEvent event);
    }
}  

---

/* DataRobot.java */
public class DataRobot {
    /* This class is for analyzing the data */
    private List _listeners = new ArrayList();
    private String data;
    public boolean analyzeData(String temp) {
        /* Analyze the data
         * This function analyzes the data, as explained in the OP
         * This function fires the analyzed data event when finished
             * analyzing the data.
         */
        data = temp;
        fireDataAnalyzedEvent(data); // this fires the dataanalyzedevent
        return true; //for now this will always return true
    }

    public synchronized void addDataAnalyzedListener(DataAnalyzedListener listener) {
        _listeners.add(listener);
    }
    public synchronized void removeDataAnalyzedListener(DataAnalyzedListener listener) {
        _listeners.remove(listener);
    }
    private synchronized void fireDataAnalyzedEvent(String temp) {
        DataAnalyzedEvent dRE = new DataAnalyzedEvent(this, temp);
        Iterator listeners = _listeners.iterator();
        while(listeners.hasNext()) {
            ((DataAnalyzedListener)listeners.next()).dataAnalyzedReceived(dRE);
        }
    }
    public interface DataAnalyzedListener {
        public void dataAnalyzedReceived(DataAnalyzedEvent event);
    }
}  

---

/* DataBuilder.java */
public class DataBuilder {
    private List _listeners = new ArrayList();
    private String data;
    public boolean submitData(String temp) {
            /* Builds the data
             * This function builds the data, as explained in the OP
             * This function fires the databuilder data event when finished
                     * building the data.
             */
        data = temp;
        fireDataBuilderEvent(data); //firing the databuilder event when finished
        return true;
    }
    public synchronized void addDataBuilderListener(DataBuilderListener listener) {
        _listeners.add(listener);
    }
    public synchronized void removeDataBuilderListener(DataBuilderListener listener) {
        _listeners.remove(listener);
    }
    private synchronized void fireDataBuilderEvent(String temp) {
        DataBuilderEvent dRE = new DataBuilderEvent(this, temp);
        Iterator listeners = _listeners.iterator();
        while(listeners.hasNext()) {
            ((DataBuilderListener)listeners.next()).dataBuilderReceived(dRE);
        }
    }
    public interface DataBuilderListener {
        public void dataBuilderReceived(DataBuilderEvent event);
    }
}  

---

/* DataSender.java */
/* this class has no event, because it is done firing events at this point */
public class DataSender {
    private String data;
    public boolean sendData(String temp) {
        data = temp;
        return true;
    }
}  

---

Below here are the event objects for each event. I Have each of this defined in a separate file, not sure if that is good procedure or not.

/* DataReceivedEvent.java */
public class DataReceivedEvent extends EventObject{
    private String data;
    public DataReceivedEvent(Object source, String temp) {
        super(source);
        // TODO Auto-generated constructor stub
        data = temp;
    }
    public String getData() {
            // this function is just an accessor function
        return data;
    }
}  

---

/* DataAnalyzedEvent.java */
public class DataAnalyzedEvent extends EventObject{
    private String data;
    public DataAnalyzedEvent(Object source, String temp) {
        super(source);
        // TODO Auto-generated constructor stub
        data = temp;
    }
    public String getData() {
            // this function is just an accessor function
        return data;
    }
}  

---

/* DataBuilderEvent.java */
public class DataBuilderEvent extends EventObject {
    private String data;
    public DataBuilderEvent(Object source, String temp) {
        super(source);
       开发者_开发问答 // TODO Auto-generated constructor stub
        data = temp;
    }
    public String getData() {
            // this function is just an accessor function
        return data;
    }
}

---

I would not say it is a "very dirty implementation". Using callbacks/observers/listeners is a good practice in my opinion.

When I write Android applications I like to layer it such that the "application" is plain old Java with no Android imports and could theoretically be used in a Swing app, a Java EE-based web site, etc. The "Android" part is strictly user interface.

What I use callbacks for is to allow the Android code to register interest in events that take place in the application. For example, in a Blackjack game, an Activity might call game.getDealer().playHand() to tell the application to perform the dealer hand play logic. As that logic executes in the application, events are fired like cardDrawn(card), cardFlipped(card), handTotalChanged(handTotal), etc. The Android part of the app listens to these and redraws things on the screen accordingly (but it knows nothing about Blackjack).

I actually just have my activities implement interfaces like CardListener, HandListener, etc. so they can receive the event directly (unlike how you do it), but your style isn't necessarily a bad way.

---

I agree with @SingleShot in theory, for the parts of your Android application that can be Android-agnostic, and so long as the overhead introduced by all the indirection layers does not slow the app down too much. IMHO, in many apps, there is relatively little that fits this description.

In another post, you proposed your above solution for one activity to communicate to another activity. In Android, activities aren't just some Java objects you can toss around willy-nilly. They are managed by the OS and have particular lifecycles. While the observer/observable pattern is quite delightful in some places, it is unsuitable where the observer/observable connection will create garbage collection problems. In particular, one activity cannot, and should not, be trying to hold some sort of listener interface on another activity.

Similarly, a clean observer/observable pattern may break down in the face of databases, threads, services, and other bits of Android reality.

So, in pure Java code, isolated from Android, what you have is probably OK. However, do not go around recommending it as solutions for Android-specific problems unless you know it will work for those Android-specific problems. And, when you start trying to make your code work in an Android app, please do not be shocked if you run into problems trying to make your textbook pattern implementation work within the constraints placed upon Android apps.