patternjavaMinor
Java HTTP Apache - Making my own library
Viewed 0 times
javaownhttplibrarymakingapache
Problem
My company has asked me to write a few Java based programs to deal with sending HTTP requests and parsing the response. After playing around with the Apache HTTP Commons library and making plenty of shortsighted programs using it, I've decided to make a little library for us to use and avoid major code duplication. We're a pretty small company working with a really old code base, and they don't seem to have time to do code reviews. So I present to you, my small API (leaving major comments outside to describe classes and break up the code segments).
```
/**
* This class facilitates an easy to use controller for creating HTTP
* requests without having a bunch of duplicate code.
*
* Typical use:
* HttpController.processBodyRequest(request, json)
*
* In retrospect, "getJSONField()" may be a little outside the scope
* of an HttpController. Meh!
*/
public class HttpController
{ private static DefaultHttpClient httpClient = new DefaultHttpClient();
public static Response processBodyRequest(Request r, String json)
throws IOException
{ r.applyHeaders();
HttpEntityEnclosingRequestBase request = r.getHttpRequest();
ByteArrayEntity dataEntity = new ByteArrayEntity(json.getBytes());
request.setEntity(dataEntity);
return executeRequest(request);
}
private static Response executeRequest(HttpEntityEnclosingRequestBase request)
throws IOException
{ HttpResponse httpResponse = httpClient.execute(request);
Header[] headers = httpResponse.getAllHeaders();
Response r = new Response(httpResponse);
for (Header h: headers)
{ r.addHeader(h.getName(), h.getValue());
}
r.setJSON(ResponseBodyDisplay._getResponseBody(httpRes
- As a small aside, you may see reference to static class
ResponseBodyDisplay, which is a simple helper class (that looks like crap) to pull out the response body. It's not posted, because I didn't write it and it looks like poop.
```
/**
* This class facilitates an easy to use controller for creating HTTP
* requests without having a bunch of duplicate code.
*
* Typical use:
* HttpController.processBodyRequest(request, json)
*
* In retrospect, "getJSONField()" may be a little outside the scope
* of an HttpController. Meh!
*/
public class HttpController
{ private static DefaultHttpClient httpClient = new DefaultHttpClient();
public static Response processBodyRequest(Request r, String json)
throws IOException
{ r.applyHeaders();
HttpEntityEnclosingRequestBase request = r.getHttpRequest();
ByteArrayEntity dataEntity = new ByteArrayEntity(json.getBytes());
request.setEntity(dataEntity);
return executeRequest(request);
}
private static Response executeRequest(HttpEntityEnclosingRequestBase request)
throws IOException
{ HttpResponse httpResponse = httpClient.execute(request);
Header[] headers = httpResponse.getAllHeaders();
Response r = new Response(httpResponse);
for (Header h: headers)
{ r.addHeader(h.getName(), h.getValue());
}
r.setJSON(ResponseBodyDisplay._getResponseBody(httpRes
Solution
I think that your exception handling is problematic.
Besides logging, all that accomplishes is degrade an "informative" kind of exception into a
If you catch an exception but aren't able to deal with it effectively, the best thing you could probably do is let it propagate — i.e., don't bother catching it at all in the first place, and declare it in the method signature if appropriate. Alternatively, if you need to avoid a leaky abstraction, you might want to wrap the exception before re-throwing it.
try {
...
} catch (IOException e) {
e.printStackTrace();
return null;
}Besides logging, all that accomplishes is degrade an "informative" kind of exception into a
null, which creates more problems than it solves:- The caller would have to explicitly check for a
nullresult. The caller cannot use a try-catch block to handle the error condition, which is the preferred way to deal with such exceptional situations.
- If the caller forgets to check for a
nullresult, then you'll probably get aNullPointerExceptionat some point. As a developer, which kind of stack trace would you rather try to diagnose: one with anIOExceptionor one with aNullPointerException?
- Suppose the caller does check for
null. What kind of feedback could it present to the user? "Sorry, your request failed for some reason. We can't tell you why." AnIOException, on the other hand, can contain more details about why the request failed. You can even choose to handle specific subclasses ofIOExceptionand ignore others.
If you catch an exception but aren't able to deal with it effectively, the best thing you could probably do is let it propagate — i.e., don't bother catching it at all in the first place, and declare it in the method signature if appropriate. Alternatively, if you need to avoid a leaky abstraction, you might want to wrap the exception before re-throwing it.
Code Snippets
try {
...
} catch (IOException e) {
e.printStackTrace();
return null;
}Context
StackExchange Code Review Q#38926, answer score: 2
Revisions (0)
No revisions yet.