Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Below is my JSON in which I have only three reportRecords just for demonstration purpose but in general sometimes we are getting pretty huge json, then it doesn't have three reportRecords only, it has large number of reportRecords.

{
   "parentRecords":{
      "reportRecords":[
         {
            "min":1.0,
            "max":1.0,
            "avg":1.0,
            "count":18,
            "sumSq":18.0,
            "stddev":0.0,
            "median":1.0,
            "percentileMap":{
               "95":1
            },
            "metricName":"TotalCount",
            "dimensions":{
               "env":"prod",
               "pool":"hawk",
               "Name":"CORE_utrade11",
               "Type":"Error"
            },
            "value":18.0
         },
         {
            "min":1.0,
            "max":1.0,
            "avg":1.0,
            "count":25968842,
            "sumSq":2.5968842E7,
            "stddev":0.0,
            "median":1.0,
            "percentileMap":{
               "95":1
            },
            "metricName":"TotalCount",
            "dimensions":{
               "env":"prod",
               "pool":"hawk",
               "Name":"ResponseHeaders",
               "Type":"ConnectionPool"
            },
            "value":2.5968842E7
         },
         {
            "min":1.0,
            "max":1.0,
            "avg":1.0,
            "count":44,
            "sumSq":44.0,
            "stddev":0.0,
            "median":1.0,
            "percentileMap":{
               "95":1
            },
            "metricName":"TotalCount",
            "dimensions":{
               "env":"prod",
               "pool":"hawk",
               "Name":"read-lookup",
               "Type":"ClientPool"
            },
            "value":44.0
         }
      ]
   },
   "minRecordsMap":{

   }
}

Now I am trying to serialize above JSON to extract those reportRecords whose Type is ClientPool and ConnectionPool only so I don't want to load everything in memory. And I am thinking to use GSON Streaming for this and I got below code working fine.

private static final List<String> metricsToExtract = Arrays.asList("ClientPool", "ConnectionPool");
// does this have to be static final?
private static final GsonBuilder gsonBuilder = new GsonBuilder();

public static void main(String[] args) {
    String urlA = "urlA";
    String urlB = "urlB";
    try {
        List<HostClientMetrics> clientMetrics = loadMetrics(urlA);
        clientMetrics.addAll(loadMetrics(urlB));
    } catch (Exception ex) {
        ex.printStackTrace();
    }
}


private static List<HostClientMetrics> loadMetrics(String url) {
    Gson gson = gsonBuilder.create();
    List<HostClientMetrics> metrics = new ArrayList<HostClientMetrics>();

    try {
        InputStream input = new URL(url).openStream();
        JsonReader reader = new JsonReader(new InputStreamReader(input, "UTF-8"));

        reader.beginObject();

        String jsonTag = null;

        while (reader.hasNext()) {
            jsonTag = reader.nextName();
            if ("parentRecords".equals(jsonTag)) {
                reader.beginObject();

                while (reader.hasNext()) {
                    jsonTag = reader.nextName();
                    if ("reportRecords".equals(jsonTag)) {
                        reader.beginArray();
                        while (reader.hasNext()) {
                            HostClientMetrics hostClientMetrics = gson.fromJson(reader, HostClientMetrics.class);
                            for (String extract : metricsToExtract) {
                                if (extract.equals(HostClientMetrics.getDimensions().getType())) {
                                    metrics.add(HostClientMetrics);
                                }
                            }
                        }
                        reader.endArray();
                    }
                }
                reader.endObject();
            } else if ("minRecordsMap".equals(jsonTag)) {
                reader.beginObject();
                // skip
                reader.endObject();
            }
        }

        reader.endObject();

        reader.close();
        return metrics;
    } catch (Exception ex) {
        System.out.println("ex:" + ex);
    }

    return metrics;
}

Below is my HostClientMetrics class

public class HostClientMetrics {

    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;

    public String getMetricName() {
        return metricName;
    }

    public Map<String, Integer> getPercentileMap() {
        return percentileMap;
    }

    public String getMedian() {
        return median;
    }

    public String getStddev() {
        return stddev;
    }

    public String getSumSq() {
        return sumSq;
    }

    public String getCount() {
        return count;
    }

    public String getAvg() {
        return avg;
    }

    public String getMax() {
        return max;
    }

    public String getMin() {
        return min;
    }

    public Dimensions getDimensions() {
        return dimensions;
    }

    public Dimensions dimensions;

    public static class Dimensions {
        private String env;
        private String pool;
        @SerializedName("Name")
        private String name;
        @SerializedName("Type")
        private String type;            

        public String getEnv() {
            return env;
        }

        public String getPool() {
            return pool;
        }

        public String getName() {
            return name;
        }

        public String getType() {
            return type;
        }           
    }
}

I am opting for code review to see whether we can improve the above code in any way which uses GSON Streaming. I need to extract those reportRecords whose Type is ClientPool and ConnectionPool only.

share|improve this question
    
+1 for being determined about streaming :) –  Sleiman Jneidi Nov 9 '14 at 20:30
    
I am downvoting this because it is apparent to me that either the above is not your code, exactly your code and nothing but your code. Or you did not get the below code working fine. Please avoid confusion and a) Copy your exact code. b) Make sure that it compiles and works. –  Simon Forsberg Nov 9 '14 at 20:34

1 Answer 1

up vote 1 down vote accepted
  • There is no point for printing an exception, it would be better if you can rethrow it, wrap it into an unchecked exception and throw it.

    catch (Exception ex) {
       throw new RuntimeException
    }
    
  • The reader will not be closed if an exception occurred while parsing the json, Java 7 supports try-with-resources statement, this will close the resources for you in a safe way

    try(JsonReader reader = new JsonReader(new InputStreamReader(input, "UTF-8")){
     ....
    }catch(Exception e){
      throw new RuntimeException(e);
    }
    

    If you aren't using Java 7 then you can go with the try-catch-finally approach.

    JsonReader reader = new JsonReader(new InputStreamReader(input, "UTF-8"));
    try{
    
    }catch(Exception e){
    throw new RuntimeException(e1);
    } finally{
     try{
     reader.close();
     }catch(Exception e1){
       throw new RuntimeException(e1);
     }
    }
    

    It's ugly isn't it? But it's the only way if you running java 6 or lower versions

  • Extract you constants into variables

    public static final REPORT_RECORDS_TAG_NAME = "reportRecords";
    
  • There is no point for declaring jsonTag string and initializing it into null outside the loop.

      String jsonTag = reader.nextName();
    
  • In your HostClientMetrics class everything is declared as a String where Java has a type system, count for instance should have a numerical type
share|improve this answer
    
Yeah I cannot move reader line only just above try block. I think inputstream one needs to be moved as well. And then I might need another try catch block for them since they throw exception as well? –  lining Nov 9 '14 at 20:42
    
@Webby When the reader get closed, the stream will be closed as well –  Sleiman Jneidi Nov 9 '14 at 20:43
    
Yeah but I am saying, I need to move these two lines above try block InputStream input = new URL(url).openStream(); JsonReader reader = new JsonReader(new InputStreamReader(input, "UTF-8")); right instead of only jsonreader one? –  lining Nov 9 '14 at 20:46
    
@Webby Yes, and why don't you try it? –  Sleiman Jneidi Nov 9 '14 at 20:47
    
I tried it already but I was saying if I move those two lines above try block, they need another try block since InputStream line throws an exception so then I need to add another try catch block just for them. And it was looking ugly. That's what I meant with my earlier comment. –  lining Nov 9 '14 at 20:50

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.