5
\$\begingroup\$

I am working on the Google Maps API, doing a reverse geocoding with the following code snippet.

What I am using are:

  • Spring
  • Jackson Parser
  • Google Maps API

It is working fine but it gives results a bit slowly. Can I improve my code in terms of performance?

package com.addressparser.api;

import java.io.IOException;
import java.io.InputStream;

import org.apache.http.HttpEntity;
import org.apache.http.HttpResponse;
import org.apache.http.client.ClientProtocolException;
import org.apache.http.client.HttpClient;
import org.apache.http.client.methods.HttpGet;
import org.apache.http.impl.client.HttpClientBuilder;

import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;

public class GeocodeAddressParser {
    public String getLocationInfo( String lat, String lng) throws JsonProcessingException, IOException {

        HttpGet httpGet = new HttpGet("http://maps.google.com/maps/api/geocode/json?latlng="+lat+","+lng+"&sensor=false");

        HttpClient httpClient = HttpClientBuilder.create().build();
        HttpResponse response;
        StringBuilder stringBuilder = new StringBuilder();

        try {
            response = httpClient.execute(httpGet);
            HttpEntity entity = response.getEntity();
            InputStream stream = entity.getContent();
            int b;
            while ((b = stream.read()) != -1) {
                stringBuilder.append((char) b);
            }
        } catch (ClientProtocolException e) {
            } catch (IOException e) {
        }

        ObjectMapper mapper = new ObjectMapper();
        JsonNode array =  mapper.readValue(stringBuilder.toString(), JsonNode.class);
        JsonNode object = array.get("results").get(0);
        String reportKey = object.get("formatted_address").textValue();
        System.out.println(reportKey);//just printing it on the console for testing at the moment
        return reportKey;

    }


    public static void main(String[] args) throws JsonProcessingException, IOException{
        GeocodeAddressParser ref = new GeocodeAddressParser();

        ref.getLocationInfo("31.338451","75.554183"); 
    }
}
\$\endgroup\$
4
  • \$\begingroup\$ Welcome to Code Review! Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers. \$\endgroup\$
    – Peilonrayz
    Jul 28, 2017 at 13:50
  • \$\begingroup\$ Oh, Ok. Sorry wasn't aware of it. I'll not update it. Thank you. \$\endgroup\$
    – Shaun
    Jul 28, 2017 at 14:17
  • \$\begingroup\$ Could you share your execution times? If up to 3 seconds, it is normal for external Internet service. \$\endgroup\$ Aug 9, 2017 at 21:00
  • \$\begingroup\$ Umm, actually execution time is 2 seconds. So it is normal not slow, right? \$\endgroup\$
    – Shaun
    Aug 10, 2017 at 15:27

2 Answers 2

1
\$\begingroup\$

Never swallow exceptions silently like this:

} catch (ClientProtocolException e) {
    } catch (IOException e) {
}

You'll have a nasty time debugging hidden problems because of that. At the very least, print some diagnostic message to System.err. Furthermore, you couldn't responsibly continue trying to parse an incomplete result, so you should abort the processing instead.

Basically, if you don't know what to do with an exception, you should probably propagate it. In fact, you already declared that getLocationInfo() can throw an IOException, so why bother catching IOException at all?

\$\endgroup\$
1
  • \$\begingroup\$ Sorry for the late reply, i saw your comment now. Yes, my bad. I have changed my code now. Thank you. \$\endgroup\$
    – Shaun
    Jul 28, 2017 at 13:41
1
\$\begingroup\$

Because this is an old post just some quick remarks:

  • The method is doing too many different things. At the very least the HTTP request and the JSON parsing should be separated.

  • There is no need to read the InputStream into a StringBuilder yourself. Jackson's ObjectMapper#readValue can read directly from an InputStream.

  • You are doing no checks, if your are actually getting the expected JSON structure. At the very least use JsonNode#path(String) and JsonNode#path(int) instead of get to avoid getting null and a NullPointerException.

EDIT: One last thing: Since you are using Spring have a look at RestTemplate. It does HTTP Requests and JSON parsing with Jackson in one step:

ObjectNode result = restTemplate.getForObject("http://maps.google.com/maps/api/geocode/json?latlng={latlang}&sensor=false", ObjectNode.class, lat+","+lng);
JsonNode object = result.get("results").get(0);
// etc.
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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