Problem
This program makes a call to an API (http://api.football-data.org/) and obtains data for fixtures of Chelsea FC for the next 100 days in JSON format. The JSON is parsed into a Java object and then displays match details in the console. I am looking for any possible improvements I could make to this program. Also, if there is a better way to parse JSON, please do mention it.
import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
import java.net.HttpURLConnection;
import java.net.MalformedURLException;
import java.net.URL;
import java.util.ArrayList;
import com.chelsea.fixtures.FixturesJsonParser;
import com.chelsea.fixtures.FixturesJsonParser.FixtureDetails;
import com.chelsea.fixtures.FixturesJsonParser.MatchResult;
import com.google.gson.Gson;
public class CfcFixture{
private static String getJson(String link){
HttpURLConnection conn = null;
try {
URL url = new URL(link);
conn = (HttpURLConnection) url.openConnection();
conn.setRequestMethod("GET");
conn.connect();
int status = conn.getResponseCode();
switch(status){
case 200:
case 201:
BufferedReader br = new BufferedReader(new InputStreamReader(conn.getInputStream()));
StringBuilder sb = new StringBuilder();
String line;
while((line = br.readLine())!=null){
sb.append(line + "n");
}
br.close();
return sb.toString();
}
} catch(MalformedURLException e){
e.printStackTrace();
}catch (IOException e) {
e.printStackTrace();
} finally{
if(conn != null){
conn.disconnect();
}
}
return null;
}
public static void main(String[] args){
// from api doc, CFC teamid is 61,
// timeFrame is next 100 days represented by n100
String link = "http://api.football-data.org/v1/teams/61/fixtures/?timeFrame=n100";
String jsonFixturesData = getJson(link);
Gson gson = new Gson();
FixturesJsonParser fixturesJP = gson.fromJson(jsonFixturesData, FixturesJsonParser.class);
System.out.println("Displaying Chelsea Fc Fixtures From " +fixturesJP.getTimeFrameStart() +
" to " + fixturesJP.getTimeFrameEnd());
System.out.println("Total Number Matches: " +fixturesJP.getNumOfGamesInTimeFrame() +"n");
for(int i=0; i<fixturesJP.getNumOfGamesInTimeFrame(); i++){
System.out.println("Match " +(i+1) +" Details");
System.out.println(fixturesJP.getFixtureDetails(i).getHomeTeamName()+ " VS. "
+fixturesJP.getFixtureDetails(i).getAwayTeamName());
System.out.println("Match Time: "+fixturesJP.getFixtureDetails(i).getMatchDate());
System.out.println("n");
}
// To get goals by home team for particular Match i.e match result
//System.out.println(fixturesJP.getFixtureDetails(1).getMatchResult().getHomeTeamGoals());
}
}
class FixturesJsonParser{
private String timeFrameStart;
private String timeFrameEnd;
private int count;
private ArrayList<FixtureDetails> fixtures;
protected String getTimeFrameStart(){
return timeFrameStart;
}
protected String getTimeFrameEnd(){
return timeFrameEnd;
}
protected int getNumOfGamesInTimeFrame(){
return count;
}
protected FixtureDetails getFixtureDetails(int matchNum){
return fixtures.get(matchNum);
}
class FixtureDetails{
private String date;
private String homeTeamName;
private String awayTeamName;
private MatchResult result;
protected String getMatchDate(){
return date;
}
protected String getHomeTeamName(){
return homeTeamName;
}
protected String getAwayTeamName(){
return awayTeamName;
}
protected MatchResult getMatchResult(){
return new MatchResult();
}
}
class MatchResult {
private String goalsHomeTeam;
private String goalsAwayTeam;
protected String getHomeTeamGoals(){
return goalsHomeTeam;
}
}
}
The JSON obtained from API is formatted as below:
HTTP/1.1 200 OK
Content-Type application/json;charset=UTF-8
X-Response-Control: minified
...
{
"timeFrameStart": "2015-10-30",
"timeFrameEnd": "2015-11-12",
"count": 2,
"fixtures":
[
{
"id": 149348,
"soccerseasonId": 405,
"date": "2015-11-03T19:45:00Z",
"matchday": 4,
"homeTeamName": "Manchester United FC",
"homeTeamId": 66,
"awayTeamName": "CSKA Moscow",
"awayTeamId": 751,
"result":
{
"goalsHomeTeam": null,
"goalsAwayTeam": null
}
},
{
"id": 146976,
"soccerseasonId": 398,
"date": "2015-11-07T15:00:00Z",
"matchday": 12,
"homeTeamName": "Manchester United FC",
"homeTeamId": 66,
"awayTeamName": "West Bromwich Albion FC",
"awayTeamId": 74,
"result":
{
"goalsHomeTeam": null,
"goalsAwayTeam": null
}
}
]
}
Solution
Use constant, avoid magic numbers
Use constant when available :
int status = conn.getResponseCode();
switch(status){
case 200:
case 201:
200 and 201 are both specific Http code so you should use a constant with a clear meaning like HttpURLConnection.HTTP_CREATED and HttpURLConnection.HTTP_OK which reaaly represent what the return code is.
Separate your code
This piece of code could have easily been a method :
BufferedReader br = new BufferedReader(new InputStreamReader(conn.getInputStream()));
StringBuilder sb = new StringBuilder();
String line;
while((line = br.readLine())!=null){
sb.append(line + "n");
}
br.close();
return sb.toString();
This is a well defined piece of code that you could extract easily and add readiblity! Doing complex stuff in a switch case is not recommend.
private String extractContentFromImput(InputStream input) throws IOException {
BufferedReader br = new BufferedReader(new InputStreamReader(input));
StringBuilder sb = new StringBuilder();
String line;
while((line = br.readLine())!=null){
sb.append(line + "n");
}
br.close();
return sb.toString();
}
Exception handling
You should try to do something useful if you’re catching exception or do not catch at all the exception. Doing e.printStackTrace(); does nothing at all. You can log it, suppress it if you don’t want to deal with it, but make it clear. At the moment the only thing you catch exception is doing is taking space.
} catch(MalformedURLException | IOException e ){
e.printStackTrace();
}
At least merge the two catch since you’re doing the same thing.
Hard-coding
This is something we all do when we try to make a small prototype and it’s really something we should always correct first. Create a config file and read it to retrieve the url. So if the url change or you want to add an argument to your url you don’t have to recompile the program.
String link = "http://api.football-data.org/v1/teams/61/fixtures/?timeFrame=n100";