You are being watched! - Comments of Interest

Question

You are being watched
Code Review has an open system
A machine that spies on you on every hour of every day
I know because I built it.
I designed the machine to detect suggestions to post on Code Review but it sees everything
Horrible comments involving ordinary users
Users like you
Comments that Stack Exchange considers irrelevant
They wouldn't act so I decided I would
But I needed some partners
Regulars with the skills to intervene
Loved by the moderators, we work in chat
You can easily find us
But on-topic or off-topic, if your comment's up
We'll find you
(Person of Interest intro, adapted to Comments of Interest)

For quite a while, users on Stack Overflow have posted comments directing people to post on Code Review. It has been noted several times on Stack Overflow Meta that users should be careful when doing that. In an effort to educate Stack Overflow users on which posts that belong here, and which does not, I decided to include the feature in my SE-chatbot named Duga.

The bot is running on a Spring MVC environment. It uses the Stack Exchange API to check Stack Overflow comments once every two minutes. It scans through the retrieved comments and posts them in The 2nd Monitor where regulars can check the Stack Overflow question to determine if it belongs on Code Review or not. It also sends some information in a special chatroom when there is a message that I am interested in - when there has been an excessive amount of comments recently or when the mysterious rate quota is reset. (I am not expecting it to be 100 comments very often. So far it has never happened within two minutes)

If you want to see an example of the API result, you can use this link.

Github repository for the whole bot can be found here

ScheduledTasks.java (relevant parts of it)

@Autowired
private ChatBot chatBot;

@Autowired
private StackExchangeAPIBean stackAPI;

private Instant nextFetch = Instant.now();
private long lastComment;
private long fromDate;
private int remainingQuota;

private final WebhookParameters params = WebhookParameters.toRoom("8595");
private final WebhookParameters debug = WebhookParameters.toRoom("20298");

@Scheduled(cron = "0 */2 * * * *") // second minute hour day day day
public void scanComments() {
    if (!Instant.now().isAfter(nextFetch)) {
        return;
    }

    try {
        StackComments comments = stackAPI.fetchComments("stackoverflow", fromDate);
        int currentQuota = comments.getQuotaRemaining();
        if (currentQuota > remainingQuota && fromDate != 0) {
            chatBot.postMessage(debug, Instant.now() + " Quota has been reset. Was " + remainingQuota + " is now " + currentQuota);
        }
        remainingQuota = currentQuota;
        List<StackExchangeComment> items = comments.getItems();
        if (items != null) {
            if (items.size() >= 100) {
                chatBot.postMessage(debug, Instant.now() + " Warning: Retrieved 100 comments. Might have missed some. This is unlikely to happen");
            }

            long previousLastComment = lastComment;
            for (StackExchangeComment comment : items) {
                if (comment.getCommentId() <= previousLastComment) {
                    continue;
                }
                lastComment = Math.max(comment.getCommentId(), lastComment);
                fromDate = Math.max(comment.getCreationDate(), fromDate);
                if (isInterestingComment(comment)) {
                    chatBot.postMessage(params, comment.getLink());
                }
            }
        }
        if (comments.getBackoff() != 0) {
            nextFetch = Instant.now().plusSeconds(comments.getBackoff() + 10);
            chatBot.postMessage(debug, Instant.now() + " Next fetch: " + nextFetch + " because of backoff " + comments.getBackoff());
        }
    } catch (Exception e) {
        logger.error("Error retrieving comments", e);
        chatBot.postMessage(debug, Instant.now() + " Exception in comment task " + e);
        return;
    }
}

private boolean isInterestingComment(StackExchangeComment comment) {
    String commentText = comment.getBodyMarkdown().toLowerCase();
    return commentText.contains("code review") || commentText.contains("codereview");
}

WebhookParameters.java

public class WebhookParameters {

    private String roomId;
    private Boolean post;

    public String getRoomId() {
        return roomId;
    }

    public void setRoomId(String roomId) {
        this.roomId = roomId;
    }

    public void useDefaultRoom(String defaultRoomId) {
        if (roomId == null) {
            roomId = defaultRoomId;
        }
    }

    public boolean getPost() {
        return post == null ? true : post;
    }

    public void setPost(Boolean post) {
        this.post = post;
    }

    public static WebhookParameters toRoom(String roomId) {
        WebhookParameters params = new WebhookParameters();
        params.setPost(true);
        params.setRoomId(roomId);
        return params;
    }

}

StackExchangeAPIBean.java

public class StackExchangeAPIBean {

    private final ObjectMapper mapper = new ObjectMapper();

    @Autowired
    private BotConfiguration config;

    public StackExchangeAPIBean() {
        mapper.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false);
    }

    public StackComments fetchComments(String site, long fromDate) throws JsonParseException, JsonMappingException, IOException {
        final String filter = "!1zSk*x-OuqVk2k.(bS0NB";
        final String apiKey = config.getStackAPIKey();
        URL url = new URL("https://api.stackexchange.com/2.2/comments?page=1&pagesize=100&fromdate=" + fromDate +
                "&order=desc&sort=creation&site=" + site + "&filter=" + filter + "&key=" + apiKey);
        URLConnection connection = url.openConnection();
        connection.setRequestProperty("Accept-Encoding", "identity");

        return mapper.readValue(new GZIPInputStream(connection.getInputStream()), StackComments.class);
    }

}

StackComments.java

public class StackComments {

    @JsonProperty
    private List<StackExchangeComment> items;

    @JsonProperty("has_more")
    private boolean hasMore;

    @JsonProperty("quota_max")
    private int quotaMax;

    @JsonProperty("quota_remaining")
    private int quotaRemaining;

    @JsonProperty
    private int backoff;

    @JsonProperty("error_id")
    private int errorId;

    @JsonProperty("error_message")
    private String errorMessage;

    @JsonProperty("error_name")
    private String errorName;

    public int getBackoff() {
        return backoff;
    }

    public int getErrorId() {
        return errorId;
    }

    public String getErrorMessage() {
        return errorMessage;
    }

    public String getErrorName() {
        return errorName;
    }

    public List<StackExchangeComment> getItems() {
        return items;
    }

    public int getQuotaMax() {
        return quotaMax;
    }

    public int getQuotaRemaining() {
        return quotaRemaining;
    }

}

StackExchangeComment.java

public class StackExchangeComment {

    @JsonProperty("post_id")
    private long postId;

    @JsonProperty("comment_id")
    private long commentId;

    @JsonProperty("creation_date")
    private long creationDate;

    @JsonProperty
    private String body;

    @JsonProperty
    private String link;

    @JsonProperty("body_markdown")
    private String bodyMarkdown;

    public String getBody() {
        return body;
    }

    public String getBodyMarkdown() {
        return bodyMarkdown;
    }

    public long getCommentId() {
        return commentId;
    }

    public long getPostId() {
        return postId;
    }

    public String getLink() {
        return link;
    }

    public long getCreationDate() {
        return creationDate;
    }

}

I am aware that WebhookParameters is not immutable, this is primarily because it can be used as URL request params in Spring MVC.

Questions

• Is there anyway I can simplify the code?
• Am I using the Stack Exchange API in a good way?
• Any other comments?

Answer 1

I have one in the "any other comments" category :)

---

Localization?

I know, that's not a concern. Or is it? Localization always seems to get left behind! Ironically, it's exactly for that reason that localizing an application is more often than not, a pain in the neck.

But then, how much harder is it, really, to write this:

if (items.size() >= 100) {
    chatBot.postMessage(debug, Instant.now() + " Warning: Retrieved 100 comments. Might have missed some. This is unlikely to happen");
}

Like that:

if (items.size() >= 100) {
    chatBot.postMessage(debug, Instant.now() + Resources.Warn100CommentsReceived);
}

I'm assuming java has something like c# resources here, apologies if I just stuck a foot in my mouth. It does look cleaner though :)

---

'100' is a magic number

That said, items.size() >= 100 isn't "100 comments received" - the message doesn't reflect what the code is doing, and this means it's ever so slightly possible that the cake message is a lie. And if you ever bust that limit, you would probably want to know by how much.

How about extracting a variable out of items.size(), and concatenating it into the message instead of the hard-coded 100?

Based the last 26 weeks of activity on Stack Overflow, the numbers would be:

• 4,320,919 comments.
• Between 116,900 and 188,280 comments - 166,190 on average, per week.
• 52-week average is 177,757 comments per week: the months ahead will be likely more busy than the ones behind us.
• That's 16.5 comments per minute, 32.97 in two minutes.

I agree that 100 is a reasonable number to use. But... '100' is a magic number! While you received 30-some comments in two minutes, at least 4-5 new users have joined Stack Overflow (~23K new users per week) - at that rate 100 may possibly, eventually, need to be replaced with a higher number. The value is completely arbitrary (why not 255?) and clearly belongs as a private static final field - that's where I'd expect to find it, if not in an application settings / configuration file.

Answer 2

Like Mat's Mug already pointed out, you have some magic numbers in your code.

private final WebhookParameters params = WebhookParameters.toRoom("8595");
private final WebhookParameters debug = WebhookParameters.toRoom("20298");  

get rid of them.

---

You shouldn't hardcode the stackexchange api url. Instead put it inside a configuration file. If the api changes so does the url. If the changes aren't breaking your application, the url maybe will.

Maybe a Url composeUrl() method would be a good idea.

---

Hardcoding the values which makes a comment interesting shouldn't be done either.

---

IMHO you are doing a little bit to much inside the scanComments() method. You are fetching the comments from the StackExchangeAPIBean, processing the remaining quota, processing the comments and calculating the next fetching by the backoff value. This should/could be extracted to separate methods.

---

If an exception is thrown by chatBot.postMessage this will also happen inside the catch part. You should better enclose the postMessage() method in a try..catch and throw an own exception, which then shouldn't be handled by calling postMessage() again.

---

The useDefaultRoom() method is a little bit strange IMHO. I would rename it to setRoomIdIfNull() but I don't see any sense in it (maybe some missing context ?).

---