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(RESPONSE).get(ERROR) == null;
    }

    public static void saveDataAsFile(JsonNode node, String dirPath, String fileName) 
            throws JsonGenerationException, JsonMappingException, IOException {

        File dir = new File(dirPath);

        if (!dir.exists()) {
            if (!dir.mkdirs()) {
                throw new IOException("Could not make file at " + dirPath);
            } 
        }

        File file = new File(dirPath, fileName);
        new ObjectMapper().writeValue(file, node);
        System.out.println("File created at: " + file);
    }

    public static boolean fetchHistoricalData(String apiKey, String city, String state, String date, String savePath) 
            throws MalformedURLException, IOException {
        // Do not attempt to get historical data unless all parameters have been passed
        if (city == null || state == null || date == null) {
            throw new IllegalArgumentException("City, State, and Date must be provided when requesting historical data");
        } 
        else {
            JsonNode json = new WundergroundData(apiKey).fetchHistorical(city, state, date);

            if (validData(json)) {
                //Files and full path will be saved in the format of ${savePath}/${city}/${date}.json
                String dirPath = String.format("%s/%s", savePath, city);
                String fileName = String.format("%s.json", date);

                saveDataAsFile(json.path(HISTORY), dirPath, fileName);

                return true;
            }
            else { 
                System.out.println(json.get(RESPONSE).get(ERROR));
                return false;
            }
        }
    }

    public static void main(String args[]) throws IOException, ParseException {

        String feature  = null;
        String city     = null;
        String state    = null;
        String date     = null;
        String apiKey   = null;
        String savePath = System.getProperty("user.dir");

        //Initialize and set up CLI help options
        Options options = new Options();
        options.addOption("f", "feature", true , "Feature requested");
        options.addOption("p", "path"   , true , "Location to save file (defaults to current working directory)");
        options.addOption("c", "city"   , true , "City requested");
        options.addOption("s", "state"  , true , "");
        options.addOption("d", "date"   , true , "Format as YYYMMDD. Date of look-up when doing a historical query");
        options.addOption("k", "key"    , true , "Wunderground API Key");
        options.addOption("h", "help"   , false, "Show help");

        //Initialize CLI Parsers
        CommandLineParser parser = new BasicParser();

        // Parse CLI input
        CommandLine cmd = parser.parse(options, args);

        // Set CLI input to variables
        if (cmd.hasOption("f")) { 
            feature = cmd.getOptionValue("f");
        }
        if (cmd.hasOption("p")) {
            savePath = cmd.getOptionValue("p") ;
        }
        if (cmd.hasOption("c")) { 
            city = cmd.getOptionValue("c");
        }
        if (cmd.hasOption("s")) { 
            state = cmd.getOptionValue("s");
        }
        if (cmd.hasOption("d")) { 
            date = cmd.getOptionValue("d");
        }
        if (cmd.hasOption("k")) { 
            apiKey = cmd.getOptionValue("k");
        }

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

API Interface Class

public class WundergroundData {
    private static final String PROTOCOL = "Http";
    private static final String WU_HOST = "api.wunderground.com";
    private String apiKey; // Wunderground requires a registered key to use services

    public void setApiKey(String apiKey) {
        this.apiKey = apiKey;
    }

    public String getApiKey() {
        return apiKey;
    }

    public URL featureUrl(String feature) throws MalformedURLException {
        // Standard Request URL Format: ${protocol}${WU_HOST}/api/${key}/${features}/${settings}/q/${query}
        return new URL(PROTOCOL, WU_HOST, String.format("/api/%s/%s", apiKey, feature));
    }

    public JsonNode fetchHistorical(String city, String state, String date)
            throws MalformedURLException, IOException {
        return new ObjectMapper().readTree(featureUrl(String.format("history_%s/q/%s/%s.json"
                                                                     , date, state, city)));
    }

    public WundergroundData() {
    }

    public WundergroundData(String key) {
        setApiKey(key);
    }
}
  • First thing I got to say is wow this is already cleaner now! Nice changes! – Marc-Andre May 7 '15 at 20:16
  • @Marc-Andre Thanks! I had help from good people :) – Jon.H May 8 '15 at 17:26
up vote 4 down vote accepted

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);
}

Your Answer

 

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

Not the answer you're looking for? Browse other questions tagged or ask your own question.