patternjavaMinor
Fetch and Save Weather Data as JSON
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
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
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
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-
When a method is strictly doing only one thing or another:
You don't need the
This also applies for all
In this case, I flipped the clauses around since the code for handling invalid JSON data is a little shorter.
And finally for your
nesting-
elseWhen 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.