Refactor attempt #3 #339
Refactor attempt #3 #339
Conversation
- Rework of the handler hierarchy to support a single base class and remove instanceof checks. - A full implementation of Cancel behaviour with granular cancellation time (downloads are now chunked so can be canceled before completion). - Progress and retry notification - Fixed some small stability issues (unhandled exception, connection issues on network switch). - SyncHttpClient fixes to use any handler (all handlers can now be forced into sync mode) - Cleanup of JsonHttpResponseHandler.java removing all superfluous helper methods Ideally JsonHttpResponseHandler.java, TextHttpResponseHandler.java, BinaryHttpResponseHandler.java should be moved into the examples directory
Conflicts: library/src/com/loopj/android/http/JsonHttpResponseHandler.java
|
This looks very nice @sweetlilmre , I'll add some inline comments where I see a boiling points. |
| responseHandler.sendResponseMessage(response); | ||
| } | ||
| } else { | ||
| throw new InterruptedException("makeRequest was interrupted"); |
There was a problem hiding this comment.
I see you removed throwing request interruption and so calling onFailed in handler subclass, how will this be handled now?
There was a problem hiding this comment.
The interruption check will cause a silent exit (as intended from cancelRequests with the mayInterruptIfRunning flag set)
For any other exceptions: they are caught in the base handler and wrapped as IOExceptions.
|
Really, just a few cosmetic changes, and I'll merge it. Well done ! |
| sendFailureMessage(status.getStatusCode(), response.getAllHeaders(), new HttpResponseException(status.getStatusCode(), status.getReasonPhrase()), responseBody); | ||
| } else { | ||
| sendSuccessMessage(status.getStatusCode(), response.getAllHeaders(), responseBody); | ||
| byte[] getResponseData(HttpEntity entity) throws IOException { |
There was a problem hiding this comment.
I see a lot of similar code in https://github.com/loopj/android-async-http/pull/339/files#diff-b7642d6783234c023a733a2f1dd10599R54
May this be merged, so it won't duplicate?
This is still a fairly large commit, I can't really get it much smaller than this, but I have tried to address your concerns as much as possible.
The major changes are generally removal of code that is no longer needed.
I have taken care to ensure that the function chains are called as they were previously to maintain backward compatibility. Additionally for the methods that I would have removed I have marked deprecated so that they can be removed at a later date.
The code has been tested using the sample application in a variety of scenarios and I have synced with your HEAD before this to ensure that I have tracked the latest changes.
Please let me know what you think.