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

Fetch and Save Weather Data as JSON

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

Problem

This is part 2 to Fetch, Parse, and Save JSON. As the code evolved, I wanted to post the program with changes for review again.

The objective of the program is to fetch JSON information from a service through their API and save it. Currently, I dislike the way the program handles bad user input in the WeatherTracker class by having two similar checks with the same error message back-to-back. I'm also beginning to feel that the job of saving the data is not necessarily the responsibility of the fetchHistoricalData function. I'm thinking that fetchHistoricalData ought to return a JSON on success, and null on failure, and another function should save the data instead. Please let me know what you think.

Lastly, in the near future there will be a script initiated by a cron job that will call this program with changing parameters at set intervals of time. I need a way for this program to exit on failure in such a way that a top level script can catch it and stop running. The only way I know how to do this now is with Sys.exit(#), but is this the best way?

Please feel free to be pendantic/critical. I would like to be able to submit this as an example of side projects I've done to future employers with confidence :D

Main Class

```
public class WeatherTracker {

private static final String RESPONSE = "response";
private static final String HISTORY = "history";
private static final String ERROR = "error";
private static final String INVALID_OPTION = "Invalid option. Please use option -h or "
+ "--help a list of available commands";
private static final String USAGE_MSG = "WeatherTracker -k [api_key] -f [feature] [-options]\n"
+ "Query Wunderground for weather data.\n The 'k' option must "
+ "be used for all feature requests";

public static Boolean validData (JsonNode node) {
return node.get

Solution

Just one suggestion for starters...

nesting-else

When a method is strictly doing only one thing or another:

private T method(Object... args) {
    if (condition) {
        // do something for true
    } else {
        // do something for false
    }
}


You don't need the else and the extra level of indentation. I will actually recommend this especially for cases where the code block can get a bit long. Therefore, you can can improve your main method as such:

public static boolean fetchHistoricalData(String... ) 
        throws MalformedURLException, IOException {
    if (city == null || state == null || date == null) {
        throw new IllegalArgumentException(...);
    }
    // no need for else here and the extra indentation
    JsonNode json = new WundergroundData(apiKey).fetchHistorical(city, state, date);
    ...
}


This also applies for all if-blocks that are returning from either branches...

JsonNode json = new WundergroundData(apiKey).fetchHistorical(city, state, date);
if (!validData(json)) {
    System.out.println(json.get(RESPONSE).get(ERROR));
    return false;
}
String dirPath = String.format("%s/%s", savePath, city);
...
return true;


In this case, I flipped the clauses around since the code for handling invalid JSON data is a little shorter.

And finally for your main() method:

if (cmd.hasOption("h") || args.length == 0) {
    new HelpFormatter().printHelp(USAGE_MSG, options);
} else if (cmd.getOptionValue("k") != null && "history".equals(feature)) {
    fetchHistoricalData(apiKey, city, state, date, savePath);
} else {
    // final catch-all
    System.out.println(INVALID_OPTION);
}

Code Snippets

private T method(Object... args) {
    if (condition) {
        // do something for true
    } else {
        // do something for false
    }
}
public static boolean fetchHistoricalData(String... ) 
        throws MalformedURLException, IOException {
    if (city == null || state == null || date == null) {
        throw new IllegalArgumentException(...);
    }
    // no need for else here and the extra indentation
    JsonNode json = new WundergroundData(apiKey).fetchHistorical(city, state, date);
    ...
}
JsonNode json = new WundergroundData(apiKey).fetchHistorical(city, state, date);
if (!validData(json)) {
    System.out.println(json.get(RESPONSE).get(ERROR));
    return false;
}
String dirPath = String.format("%s/%s", savePath, city);
...
return true;
if (cmd.hasOption("h") || args.length == 0) {
    new HelpFormatter().printHelp(USAGE_MSG, options);
} else if (cmd.getOptionValue("k") != null && "history".equals(feature)) {
    fetchHistoricalData(apiKey, city, state, date, savePath);
} else {
    // final catch-all
    System.out.println(INVALID_OPTION);
}

Context

StackExchange Code Review Q#90075, answer score: 4

Revisions (0)

No revisions yet.