HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Twitter Streaming API Client: Identifying the top trending hashtags for a specific term

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thetophashtagstermtrendingclientstreamingforspecifictwitter

Problem

I’ve been experimenting with the Twitter Streaming API and would like some critical feedback on my latest project. Specifically code correctness, code smells, overall structure, and my usage of collections. The application identifies the current top trending hashtags for the supplied hashtag, or string.

AbstractClient.java

package com.gmail.lifeofreilly.lotus;

/**
 * An Abstract client for retrieving messages that contain hashtags. Can be extended for target social network.
 */
public abstract class AbstractClient implements Runnable {
    private String trackedTerm;
    private String screenName;
    private Long id;
    private MessageData messageData;

    public MessageData getMessageData() {
        return messageData;
    }

    public void setMessageData(final MessageData messageData) {
        this.messageData = messageData;
    }

    public String getTrackedTerm() {
        return trackedTerm;
    }

    public void setTrackedTerm(final String trackedTerm) {
        this.trackedTerm = trackedTerm;
    }

    public String getScreenName() {
        return screenName;
    }

    public void setScreenName(final String screenName) {
        this.screenName = screenName;
    }

    public Long getID() {
        return id;
    }

    public void setId(final Long id) {
        this.id = id;
    }

    @Override
    public String toString() {
        return "Client{" +
                "trackedTerm='" + trackedTerm + '\'' +
                ", screenName='" + screenName + '\'' +
                ", id=" + id +
                '}';
    }
}


MessageData.java

```
package com.gmail.lifeofreilly.lotus;

import org.apache.log4j.Logger;

import com.google.common.collect.Multiset;
import com.google.common.collect.Multisets;
import com.google.common.collect.TreeMultiset;

import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.LinkedList;
import java.util.Map;
import java.util.Queue;
import java.util.Set;

/**
* A message queue and the hashtag

Solution

Recommendation: use an ExecutorService instead of managing the threads yourself.

You should inject Dependencies, like TwitterClient, into the classes that need them, rather than doing the instantiation locally. If you feel like to absolutely need to hide the implementation detail that is the object graph -- well that's what factories are for.

Similarly, you should inject the dependency you need - if TwitterListener needs to interact with MessageData, then it should be initialized with a reference to it, rather than borrowing the instance from TwitterClient each time. Hard to say - you are inconsistent about whether the MessageData is final (MessageProcessor) or not (AbstractClient). It's not at all clear how this system is supposed to work if everybody isn't sharing the same data.

The TwitterClient constructor is much too busy, and the call to System.exit() is a real shocker. If you were injecting the dependencies you need, you wouldn't need that surprise.

I'm very suspicious of the messageQueue logic, and the locking strategy that you are using. If TwitterClient and MessageProcessor are trying to exchange data via MessageData, then I think all of the synchronization should be inside the MessageData class itself.

Instead of having the timer invoke the getTopTenHashtags(), I'd probably arrange to signal the MessageProcessor thread to invoke it. That protects you from some of the consistency headaches.

You might look at using the Disruptor pattern, instead of trying to work through all of the synchronization issues yourself.

Context

StackExchange Code Review Q#52014, answer score: 6

Revisions (0)

No revisions yet.