Serializing JSON data coming from two URLs in the same object

Question

I have two URLs (urlA and urlB) and both the URL gives me a JSON response back in the same JSON format. Below is my JSON string which I am getting back by calling from urlA. I have shorten it down by having only three reportRecords for the understanding purpose. In general, it might have more than ~500 reportRecords.

{
 "aggRecords": {
 "reportRecords": [
 {
 "min": 0,
 "max": 12,
 "avg": 0.3699187,
 "count": 246,
 "sumSq": 571,
 "stddev": 1.4779372,
 "median": 0,
 "percentileMap": {
 "95": 4
 },
 "metricName": "TransactionDuration",
 "dimensions": {
 "env": "dev",
 "pool": "titan",
 "Name": "PostProcessing",
 "Type": "PostProcessing"
 },
 "value": 91
 },
 {
 "min": 0,
 "max": 23,
 "avg": 2.3991289E-4,
 "count": 1463031,
 "sumSq": 3071,
 "stddev": 0.045814946,
 "median": 0,
 "percentileMap": {
 "95": 0
 },
 "metricName": "TransactionDuration",
 "dimensions": {
 "env": "dev",
 "pool": "titan",
 "Name": "ResourceContext",
 "Type": "ResourceContext"
 },
 "value": 351
 },
 {
 "min": 0,
 "max": 1209,
 "avg": 1.9203402,
 "count": 7344636,
 "sumSq": 71832774,
 "stddev": 2.4683187,
 "median": 2,
 "percentileMap": {
 "95": 4
 },
 "metricName": "TransactionDuration",
 "dimensions": {
 "env": "dev",
 "pool": "titan",
 "Name": "Client::Sync",
 "Type": "Client::Sync"
 },
 "value": 14104200
 }
 ]
 }
}

Similarly I am also getting another JSON response back by calling urlB and it is in the exact same JSON format as shown above but the values for all the keys are different.

Now what I need to do is, I need to serialize JSON response coming from both the URL's (urlA and urlB) in DataTransactionMetrics object only and then iterate DataTransactionMetrics object and extract relevant information from it in which I am interested in it. Meaning, I will be extracting few Name values from the DataTransactionMetrics object in which I am interested in it and populate it into hostMetricsList object and at the end print out the full list.

Below is my full code which does the JSON serialization of both the URL's -

public class JSONParser {
 private static RestTemplate restTemplate = new RestTemplate();
 private static final Gson gson = new Gson();
 private static final Type listOfMetricsType = new TypeToken<List<DataTransactionMetrics>>() {
 }.getType();
 private static final String BASE_URL = "http://data.host.com/reports/";
 private static final String TRANSACTION_METRIC = "/Transaction?defaultDim=Name&defaultDim=Type&metric=TransactionDuration&percentile=95";
 private static final String EVENT_METRIC = "/Event?defaultDim=Name&defaultDim=Type&metric=TotalCount&percentile=95";
 private static final List<String> hostNames = Arrays.asList("hostA", "hostB", "hostC");
 public static void main(String[] args) throws IOException {
 List<HostMetrics> hostMetricsList = new ArrayList<HostMetrics>();
 for (String hostName : hostNames) {
 HostMetrics hostMetrics = new HostMetrics();
 hostMetrics.sethostName(hostName);
 Calendar startDate = new GregorianCalendar();
 startDate.set(Calendar.MINUTE, 0);
 Calendar endDate = (Calendar) startDate.clone();
 startDate.set(Calendar.HOUR_OF_DAY, 0);
 DateFormat df = new SimpleDateFormat("yyyy/MM/dd HH:mm");
 String startTime = df.format(startDate.getTime());
 String endTime = df.format(endDate.getTime());
 // construct urlA
 String urlA = BASE_URL + hostName + TRANSACTION_METRIC + "&startTime=" + startTime
 + "&endTime=" + endTime;
 // construct urlB
 String urlB = BASE_URL + hostName + EVENT_METRIC + "&startTime=" + startTime
 + "&endTime=" + endTime;
 // serialize everything in DataTransactionMetrics object, both the URL's
 List<DataTransactionMetrics> dataMetrics = loadMetrics(urlA);
 dataMetrics.addAll(loadMetrics(urlB));
 // and then extract only few fields from it and populate in HostMetrics object
 for (DataTransactionMetrics metrics : dataMetrics) {
 if (metrics.getDimensions().getName().equalsIgnoreCase("DataSync")) {
 hostMetrics.setTotalNumberOfSyncCall(metrics.getCount());
 hostMetrics.setSyncNinetyFivePercentileCall(String.valueOf(metrics.getPercentileMap().get("15")));
 hostMetrics.setSyncAvgCall(metrics.getAvg());
 hostMetrics.setSyncMaxCall(metrics.getMax());
 } else if (metrics.getDimensions().getName().equalsIgnoreCase("DataAsync")) {
 hostMetrics.setTotalNumberOfAsyncCall(metrics.getCount());
 hostMetrics.setAsyncNinetyFivePercentileCall(String.valueOf(metrics.getPercentileMap().get("15")));
 hostMetrics.setAsyncAvgCall(metrics.getAvg());
 }
 }
 hostMetricsList.add(hostMetrics);
 }
 // and then print out the list
 System.out.println(hostMetricsList);
 }
 private static List<DataTransactionMetrics> loadMetrics(String url) {
 String jsonString = restTemplate.getForObject(url, String.class);
 JsonObject json = new JsonParser().parse(jsonString).getAsJsonObject();
 JsonArray jarr = json.getAsJsonObject("aggRecords").getAsJsonArray("reportRecords");
 return gson.fromJson(jarr, listOfMetricsType);
 }
}

And below is my DataTransactionMetrics class -

public class DataTransactionMetrics {
 private String metricName;
 private Map<String, Integer> percentileMap;
 private String median;
 private String stddev;
 private String sumSq;
 private String count;
 private String avg;
 private String max;
 private String min;
 // getters here
 public Dimensions dimensions;
 class Dimensions {
 private String env;
 private String pool;
 @SerializedName("Name")
 private String name;
 // getters here
 }
}

And below is my HostMetrics class -

public class HostMetrics {
 private String hostName;
 private String totalNumberOfSyncCall;
 private String syncNinetyFivePercentileCall;
 private String syncAvgCall;
 private String syncMaxCall;
 private String totalNumberOfAsyncCall;
 private String asyncNinetyFivePercentileCall;
 private String asyncAvgCall;
 // getters and setters here
} 

Now I have got everything working in the above code. I am opting for code review to see whether we can improve anything here in the code.

• TheCoffeeCup: Why don't you add Exception Handling?
• arsenal: @MannyMeng I already have added exception handling. Sorry I should have edited my question earlier.

Answer 1

Your constructor should create usable objects

Make sure that the constructor always creates a usable object. This saves your class's clients the trouble from debugging missing calls to init or set methods that are required.

Basically, if a method call is required before the class is usable, then that should be in the constructor.

Your HostMetrics always has to have sethostName called or it won't function as it appears to me. The correct way to do this would be to remove the sethostName function and make the host name a constructor argument.

Extract methods

You could extract a constructMetricUrl method in the HostMetrics class to create the urls and get rid of the comments here:

 // construct urlA
 String urlA = BASE_URL + hostName + TRANSACTION_METRIC + "&startTime=" + startTime
 + "&endTime=" + endTime;
 // construct urlB
 String urlB = BASE_URL + hostName + EVENT_METRIC + "&startTime=" + startTime
 + "&endTime=" + endTime;

In general if you need a comment to say what something does, there is a good chance that that something should be a function call. The function name also serves as an automatic comment and improves readability.

Without having more information about the DataTransactionMetrics and HostMetrics classes I think that you should be able to move some of the functionality in your main() method into member functions on these classes. A general rule of thumb is that if your function doesn't quite fit comfortably on your screen then the function is too long and should be broken up into sub-routines (or you're in dire need a of a bigger screen).

In essence with proper designed classes your main function would look something like this:

public static void main(String[] args) throws IOException {
 List<HostMetrics> hostMetricsList = new ArrayList<HostMetrics>();
 for (String hostName : hostNames) {
 Calendar startDate = new GregorianCalendar();
 startDate.set(Calendar.MINUTE, 0);
 Calendar endDate = (Calendar) startDate.clone();
 startDate.set(Calendar.HOUR_OF_DAY, 0);
 HostMetrics hostMetrics = new HostMetrics(hostName, startDate, endDate);
 hostMetricsList.add(hostMetrics);
 }
 // and then print out the list
 System.out.println(hostMetricsList);
}

Storing results

In your main you're storing the hostMetricsList only to print it when you're done. Why not simply print the metrics as you go and avoid the list all together?

• Dan: I wouldn't say putting an object into a usable state is RAII - that's just what a constructor should be used for - and initialize methods are not a symptom of not using RAII but not understanding the purpose of constructors IMO. Don't get me wrong, promoting RAII is great and all, but what you've suggested here isn't really using RAII - RAII is is the idea that when you acquire an object you acquire all of it's dependant resources, and when you deterministically delete the object all of it's resources are released.
• Emily L.: True, I went overboard with RAII. But generally I see a "initialize" method as a codesmell.
• Dan: That I agree with :P
• arsenal: @EmilyL. Thanks a lot for your suggestion. Sorry I was out on vacation for a week so not able to work on your suggestion. I have started working now and your suggestion looks great to me. One thing I am not clear is as you said, I can make constructMetricUrl method in HostMetrics class itself but I am not sure how would that method be called and from where in HostMetrics class? And also where I would add my serialization code example as shown in the question. Can you provide an example how would my HostMetrics class would look like?
• Emily L.: You need the metric URL in all cases as far as I can tell so you can just call it from the constructor of HostMetrics. After the constructor is executed the HostMetrics most of the work would be done already. You would just basically move most of the code in the main method to the constructor of HostMetrics and preferrably split it into smaller functions that are easy to understand.

Answer 2

In addition to what Emily L. says, make class Dimensions static or move it into its own file as a top level class.

If this applies depends on your serialisation mechanism, still:

If you don't, a new additional Dimensions class will be loaded for each instance of DataTransactionMetrics.

Loading and garbage collecting classes is no small task. The application performance will probably improve if you make it static.