Sign up ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I'm using Struts2 and Google App Engine. Below is my program for getting the country based on the IP of the visitor of the website.

In my Struts2 action:

HttpServletRequest request = ServletActionContext.getRequest();
String IP = request.getRemoteAddr();
String countryCode = Utilities.fetchUrl( "" + IP );
if ( countryCode != null){
    String country = CountryUtil.COUNTRY_MAP.get( countryCode );
    System.out.println("COUNTRY: " + country);

My fetchUrl static function:

public static String fetchUrl(String strUrl){
    String output = "";
    String line = null;
    try {

        URL url = new URL( strUrl );
        BufferedReader reader = new BufferedReader(new InputStreamReader(url.openStream()));
        while ((line = reader.readLine()) != null) {
            output += line;

    } catch (MalformedURLException e) {
        System.out.println("ERROR CATCHED: " + e.getMessage());
        return null;
    } catch (IOException e) {
        System.out.println("ERROR CATCHED: " + e.getMessage());
        return null;

    return output;


public static final Map<String, String> COUNTRY_MAP;
static {
    Map<String, String> map = new HashMap<String, String>();
    map.put("AF", "afghanistan");
    map.put("AL", "albania");
    map.put("ZW", "zimbabwe");
    COUNTRY_MAP = Collections.unmodifiableMap( map );

Any suggestion on improving its speed? I also need suggestions for the coding.

share|improve this question
Have a look in , and at Item 33 of Joshua Bloch's Extreme Java book – cl-r Feb 25 '13 at 7:34
Not related to speed: reader.close(); should be in a finally-block. – MrSmith42 Feb 25 '13 at 10:57

3 Answers 3

up vote 7 down vote accepted

The slowest part will be the request to the remote server. There is not much you can do to make this faster. You may want to cache the results you get back from hostip, or you may want to have a local database.

Furthermore, the API seems to return XX if it does not know the country, you may want to check for that.

Also, you do not close the InputStream or the InputStreamReader, and it is possible that you do not close the BufferedReader when an exception occurs. I don't know exactly whether this will cause any problems.

share|improve this answer

I would agree to the comment Answer from @Sjoerd. The slowest part is the http request. You could save some very little time to access this in other ways, but most probably, the packet transfer times are much higher than everything else.

Beside this, you should clean up your utility method and make it more robust.
I would suggest to create a separate class, the constructor takes the ip as an argument. The class has a read or updateData method and has getters like getCountryName(), getCountryCode(), getIp() and so one.
Then you could use one of this the json or the xml:

And you have all information directly from the result. You should not create the map by youself, because you will easily forget something (my result was EU, you got this inside the map?) and you will never update it again.

Next step would be to handle your exceptions in a proper way. You should have finaly blocks as already suggested and you should handle errors. If you have your own update method, it could return a boolean indicating the success. And so one.

share|improve this answer

The question is old, and has already valuable answers.

By the way, two main adjustments can be done to drastically reduce the loading times:

  1. Use a faster webservice:

    The timings are taken from Italy, but I guess the second webservice is faster no matter from where it's called.
    It's a 600% performance improvement, so it's worthy of a try.
    It can also be used with JSONP to get other useful data, eg. geolocalization, as shown in this demo.

  2. Cache the data

    Your server should call this webservice only once per-user. Then you can store the result (and retrieve it later instead of performing a new call), mainly in:

    • user's Session: it will automatically expire and is already isolated;
    • a centralized "timed" Map, manually handling the expiration of the values (eg. once a day, or once a hour, depending on how many users you have and how many RAM your server has).

With the two small changes described avoce, the loading time per-user would change from:

~ 300 ms
~ 300 ms
~ 300 ms
~ 300 ms


~ 50 ms
~ 0 ms
~ 0 ms
~ 0 ms

, that sounds better.

share|improve this answer

Your Answer


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.