Fetch and Save Weather Data as JSON

Question

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

• Marc-Andre: First thing I got to say is wow this is already cleaner now! Nice changes!
• Jon.H: @Marc-Andre Thanks! I had help from good people :)

Answer 1

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