Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Return Twitter4j classes instead of Attachments #9

Open
gdaniel opened this issue Jan 13, 2021 · 2 comments
Open

Return Twitter4j classes instead of Attachments #9

gdaniel opened this issue Jan 13, 2021 · 2 comments
Assignees
Labels
enhancement New feature or request

Comments

@gdaniel
Copy link
Member

gdaniel commented Jan 13, 2021

This issue concerns the following classes:

  • GetTrends
  • LookForTweets
  • ReceiveDM

The compute() method of the classes listed above returns a List<Attachment> representing the information they retrieve from Twitter. Attachment is a class from the library we use to access Slack, this means that the Twitter platform is tightly coupled to the Slack platform.

While this made some sense in the previous versions of Xatkit (mostly because of the limitations of the language), I don't think it is desirable right now. We should decouple the Twitter platform from the Slack one, and let bot designers create any kind of bot interacting with Twitter.

The solution I see is to remove the mapping logic from Twitter4j classes to attachments. For example the compute method in GetTrends could be updated as follows:

@Override
protected Object compute() {
    String result = "0";
    Twitter twitterService = this.runtimePlatform.getTwitterService();
    List<Trend> trendsList = new ArrayList<>();

    // First WOEID is 1 which correspond to "Worldwide"
    if (woeid > 0) {
        try {
            Trends trends = twitterService.getPlaceTrends(woeid);
            if (trends.getTrends().length > 0) {
                for (Trend trend : trends.getTrends()) {
                    trendsList.add(trend);
               }
            }
        } catch (TwitterException e) {
            result = "1";
            e.printStackTrace();
        }
    }

    return trendList;
}

(This is just an example, it may make sense to directly return the Trends instance instead of creating a list).

With this kind of implementation bot designers can call this action in any context (Slack or not). Of course this also means that formatting Twitter4j classes to attachments is not done automatically anymore. I think it's fine, and we can provide some formatters later.

@gdaniel gdaniel added the enhancement New feature or request label Jan 13, 2021
@nerlichman nerlichman self-assigned this Jan 14, 2021
@nerlichman
Copy link
Collaborator

I completely agree, now it makes more sense to return Twitter4J objects.

On the other hand, it makes it much more complex to post that data in Slack (which now is as direct as it gets). One option would be to have a cheet sheet (in wiki or readme, for example) that shows how to convert the Twitter4J objects to something that Slack can render (basically copying the formatting code that today is in the platform).

@gdaniel what are your thoughts on this?

@gdaniel
Copy link
Member Author

gdaniel commented Jan 14, 2021

I think it's a great idea! You can even create a Gist for it if you think it's more convenient. I'd put it in the readme, probably towards the end as an additional resource, so people don't get that in their face if they aren't actually trying to use the Twitter platform with Slack

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants