Executing URL using Apache HttpClient

Question

I have XML below which is stored as a String in xmlData variable and I need to pass this String to my URL in client_data variable and then execute the URL:

<?xml version="1.0"?>
<ClientData
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" 
 xsi:schemaLocation="http://www.google.com model.xsd" 
 xmlns="http://www.google.com"> 
    <client id="100">
        <clock>
                <for>
                    <etc>val(tery) = 1</etc>
                    <while><![CDATA[val(tery)  < 20]]></while>
                </for>
        </clock>
    </model>
</ClientData>

Below is the URL I am hitting on the browser by URL encoding client_data value manually to get the response back and it works fine through browser -

http://localhost:8080/test_tmp?max_time=30&users=1000&client_data=<?xml version="1.0"?>
<ClientData
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" 
 xsi:schemaLocation="http://www.google.com model.xsd" 
 xmlns="http://www.google.com"> 
    <client id="100">
        <clock>
                <for>
                    <etc>val(tery) = 1</etc>
                    <while><![CDATA[val(tery)  < 20]]></while>
                </for>
        </clock>
    </model>
</ClientData>

Now I have started using Apache HttpClient to execute the URL and I have got below code which is working fine -

Using Apache HttpClient

This code works fine and I am able to get the response back. I am getting JSON response back as a string which is expected.

public static void main(String[] args) {
    for(int i= 1; i<=100; i++) {
        // this will return xml as a string
        String xmlData = getXMLData(i);
        String url = generateURL(xmlData);

        // do I need to create this everytime for each call?
        HttpClient httpclient = HttpClientBuilder.create().build();
        HttpGet request = new HttpGet(url);
        StringBuilder response = new StringBuilder();

        try {
            HttpResponse httpResponse = httpclient.execute(request);
            StatusLine statusLine = httpResponse.getStatusLine();
            int statusCode = statusLine.getStatusCode();
            String phrase = statusLine.toString();

            if (statusCode == HttpStatus.OK.value()) {
                BufferedReader rd = new BufferedReader(new InputStreamReader(httpResponse.getEntity().getContent()));

                String line = "";
                while ((line = rd.readLine()) != null) {
                    response.append(line);
                }
            }
        } catch (ClientProtocolException e) {
            System.out.println(e.getMessage());
        } catch (IOException e) {
            System.out.println(e.getMessage());
        }

        // prints response
        System.out.println(response.toString());

        // and then I am serializing json "response" here to my POJO if it not null
    }
}

private static String generateURL(final String clientData) {
    StringBuilder url = new StringBuilder();
    url.append("http://localhost:8080/test_tmp?max_time=30&users=1000&client_data=");
    url.append(URLEncoder.encode(clientData, "UTF-8"));

    return url.toString();
}

I am opting for code review to see whether there is any better way to execute the URL efficiently using Apache HttpClient or if there is any way to simplify the above code.

Answer 1

Efficiency

In response to

// do I need to create this everytime for each call?
HttpClient httpclient = HttpClientBuilder.create().build();

Then no. The construction doesn't take any parameter, so it's independent from the loop and you can initialize only once before the loop. And although the javadoc for those methods is empty, by the looks of .create().build() I'd say that it's not a cheap instantiation.

Simplification

You can save some lines by using IOUtils to read the content from the response, as explained in this answer.

Readability

Your function is a bit too long. You can probably divide it into smaller functions. I would start extracting the try/catch block into a readResponse() method.

Miscellaneous

This

HttpStatus.OK.value()

is from Spring. There's nothing wrong with it, but since you're using an Apache library, why don't you use HttpStatus from the Apache library?