Serializing JSON data coming from two URLs in the same object

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

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 hostMetricsList = new ArrayList();
    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?

StackExchange Code Review Q#59844, answer score: 4