Simon, Mat's Mug, what's the (rating) difference?

This is a part of the code for the almighty bot @Duga (That's me!).

Every evening, 15 minutes before "RELOAD" (00:00 UTC), I post the reputation differences for some Code Review users.

Example output is:


Mat's Mug vs. Simon Forsberg: 2728 diff. Year: +3618. Quarter: +885. Month: +648. Week: -40. Day: +185.

The code below makes use of the following classes/interfaces:

• StackExchangeAPI: Contains methods for doing a request to Stack Exchange API. The result is then parsed and "slurped" as JSON and stored into a Groovy object.

• DugaBot: Contains methods for sending a message to chat

• WebhookParameters: Information about where to send the chat message (which chat room)

This class is created on startup (or when tasks are reloaded) and then run() is called on this class through a cron trigger in Grails.

Any comments about the code appreciated.

This code is also available on Github: Zomis/Duga.

```
class UserRepDiffTask implements Runnable {

private final StackExchangeAPI stackApi;
private final DugaBot chatBot;
private final String usersString;
private final String site;
private final WebhookParameters room;

public UserRepDiffTask(StackExchangeAPI stackApi, String room, DugaBot chatBot, String users, String site) {
this.stackApi = stackApi;
this.chatBot = chatBot;
this.usersString = users.replace(',', ';');
this.room = WebhookParameters.toRoom(room);
this.site = site;
}

@Override
public void run() {
try {
def result = stackApi.apiCall("users/" + usersString, site, "!23IYXA.sS8.otifg5Aq.2");
List users = result.items
if (users.size() != 2) {
throw new UnsupportedOperationException("Cannot check diff for anything other than two users");
}

def max = users.stream().max(Comparator.comparingInt({it.reputation})).get();
def min = users.stream().min(Comparator.comparingInt({it.reput

-
The static chatName() method isn't used at all. Just get rid of it because it is only adding noise.

-
Why is the static clearName() method public ? Make it either private and non static or place it in some other class which has the responsibility to do such cleaning to get the clear name.

Or to ask different, what is wrong with using org.apache.commons.lang.StringEscapeUtils.unescapeHtml like you do in AnswerInvalidationCheck ?

-
StringBuilder

-
IMO if you "know" or if you can narrow the resulting capacity of the StringBuilder you should use the overloaded constructor which takes the initial capacity as a parameter.

-
I like the fluent methods of the StringBuillder. IMO taking advantage of the fluent way makes it easier to read the code.

-
doing string concatenation to build a parameter for the append() method is somehow strange. Why don't you just use the append() method ?

-
Some vertical spacing would be good for your code. It groups related code and makes it more readable.

def max = users.stream().max(Comparator.comparingInt({it.reputation})).get();
def min = users.stream().min(Comparator.comparingInt({it.reputation})).get();

StringBuilder str = new StringBuilder();
str.append(clearName(max.display_name) + " vs. " + clearName(min.display_name) + ": ");
str.append((int)max.reputation - (int)min.reputation);
str.append(" diff. ");

diffStr(str, max, min, "Year", {it.reputation_change_year});
diffStr(str, max, min, "Quarter", {it.reputation_change_quarter});
diffStr(str, max, min, "Month", {it.reputation_change_month});

StackExchange Code Review Q#114328, answer score: 11