Badger - The tumbleweed detector

I have written a small program to query the Stack Exchange API, specifically for badges. The main idea of the program is to return the new list of Tumbleweed posts on Stack Overflow, every 10 minutes. The motivation for this bot was the Weed Eater hat.

The program queries the API every 10 mins. If there are new tumbleweed posts, Then it'll print the number of new posts with a link to the tumbleweed page. If there are no new ones, It waits for another 10 minutes.

The code for the same is

```
import com.google.gson.JsonArray;
import com.google.gson.JsonObject;
import com.google.gson.JsonParser;
import org.jsoup.Connection;
import org.jsoup.Jsoup;
import java.io.IOException;
import java.time.Instant;

public class RunBadger {

private static final String apiKey = "my api key";
public static Instant previousBadgeTimestamp = Instant.now();
public static void main(String[] args) {
try {
while (true) {
JsonArray weeds = getBadges().get("items").getAsJsonArray();
int numberOfWeeds = weeds.size();
if(numberOfWeeds!=0) {
System.out.println(" [Badger ] " + numberOfWeeds + " new Tumbleweed posts");
}
previousBadgeTimestamp = Instant.now();
Thread.sleep(600000);
}
}
catch (Exception e){
e.printStackTrace();
}

}
public static JsonObject getBadges() throws IOException{
String badgeIdUrl = "https://api.stackexchange.com/2.2/badges/63/recipients";
JsonObject badgeJson = get(badgeIdUrl,"site","stackoverflow","pagesize","100","fromdate",String.valueOf(previousBadgeTimestamp.minusSeconds(1).getEpochSecond()),"key",apiKey);
return badgeJson;
}
public static JsonObject get(String url, String... data) throws IOException {
Connection.Response respons

Conceptually, when you want to perform an action at a fixed rate, you want to use an ScheduledExecutorService. This class is designed to handle the use-case of running code at a fixed rate, possibly with an initial delay. The advantage is that you can schedule more than one task, and it also lets you handle all the tasks, eventually cancelling some of them. It's not so much that the while(true) is an issue per se, but doing all those operations with that approach would be a lot more complicated. Furthermore, it wouldn't really let you perform other unrelated actions at the same time as the code getting the badges. Additionally, don't use the Timer class, and always prefer a ScheduleExecutorService, when you can use one.

Rewriting the code to use the service would actually make it more clear. First of all, you want a method that represents the action to perform:

private static void printBadges() {
    JsonArray weeds;
    try {
        weeds = getBadges().get("items").getAsJsonArray();
    } catch (IOException e) {
        e.printStackTrace(); // this should be replaced with a real logger
        return;
    }
    int numberOfWeeds = weeds.size();
    if(numberOfWeeds!=0) {
        System.out.println("[ [Badger](https://www.youtube.com/watch?v=dQw4w9WgXcQ) ] " + numberOfWeeds + " new [Tumbleweed posts](//stackoverflow.com/help/badges/63/tumbleweed)");
    }
    previousBadgeTimestamp = Instant.now();
}

With that method in place, scheduling the task is as easy as

ScheduledExecutorService executorService = Executors.newSingleThreadScheduledExecutor();
executorService.scheduleAtFixedRate(RunBadger::printBadges, 0, 10, TimeUnit.MINUTES);

This documents clearly what is happening, mainly that something scheduled at a fixed rate, and the delay is obvious: 10 minutes. The second parameter is a potential initial delay before performing the action the first time, which I set to 0 here.

A note about the exception handling. The current code catches the exception outside of the while loop, which means that any exception will stop the program from running; generally, this isn't want you want. An exception from getting the badges could occurs for multiple reasons and it shouldn't stop the rest of the code from running (although in this case, there is no other part of the code, but imagine this in a more complex scenario, with other tasks running concurrently). The scheduledAtFixedRate method will suppress the execution of future tasks, thereby cancelling the schedule, if there is an exception thrown by the task: this means that if an exception is thrown, nothing will get executed anymore. Catching and handling the exceptions inside the task, like done in the example above, lets you manage this.

The question of what should happen after scheduling the task remains. If you just want to run it for fun until you kill the process manually, you can just keep this in the main method:

public static void main(String[] args) {
    ScheduledExecutorService executorService = Executors.newSingleThreadScheduledExecutor();
    executorService.scheduleAtFixedRate(Main::printBadges, 0, 1, TimeUnit.SECONDS);
}

The executor service won't be garbage collected and the action will run every 10 minutes, until you kill the JVM yourself. A cleaner approach would be to invoke executorService.shutdown(); when you want the process to stop, as it will correctly stop the executor, and let the main thread finish properly. (Note that it won't block until the tasks are finished; awaitTermination can be used for that). You just need to find the stopping condition, like a specific exception thrown in the task. It could be added as a runtime hook as well, but note that those hooks don't run when killing with SIGKILL.

Other thoughts:

JsonObject root = new JsonParser().parse(json).getAsJsonObject();
return root;

You don't need to store this in a temporary variable just before returning. Returning directly is just as clear, and doesn't introduce a variable.

return new JsonParser().parse(json).getAsJsonObject();

public static JsonObject getBadges() throws IOException

This may tie a bit the code fetching the badges to the code printing the result. In fact, you're only interested in the number of new items, so make that method return the count directly:

public static int getBadgesCount() throws IOException

This way, the rest of the code is hidden from the internal JSON representation of what the API returns.

public static Instant previousBadgeTimestamp

That's very good use of Instant: it represents a point in the timeline in UTC and isn't tied to any specific timezone. When you want to model at what time a system event took place, Instant is the perfect class.

private static final String apiKey = "my api key";

If the API key deserves its constant, why not all the other hard-coded Strings like badgeIdUrl or the rick-rolling very interesting link badge

StackExchange Code Review Q#148570, answer score: 14