First of all, thanks to Jack (off-line) and Endi for the review. Attached is a replacement patch that supersedes the original patch. One main thing with this patch is that I took jack's suggestion and looked into possibility of adding uri after the HTTP Connection is created. This is different from the original ca->kra case where the uri is set at the time when HttpConneciton was created (originally, I was trying to preserve that). After looking carefully into it, I found it possible. Therefore, now we keep the connection unattached to any uri, op is no longer needed during the time when HttpConnection was created or retrieved. Before send, however, the uri will need to be set, and that's when op is used to retrieve the uri. I find this approach a lot cleaner. Although due to the change of the approach, many of Endi's comments do not immediately apply to this new patch, I was able to apply indirectly some of the concepts into the code in this patch. Please review. thanks, Christina On 03/24/2014 02:32 PM, Endi Sukma Dewata wrote:
On 3/24/2014 4:22 PM, Endi Sukma Dewata wrote:
4. In getConnForOp() if op is null it will remove an element but will still return a null:
private IHttpConnection getConnForOp(String op) { IHttpConnection retConn = null; if (op == null) { retConn = mConns.elementAt(mNumConns); mConns.removeElementAt(mNumConns); } else { ... } return retConn; // return null? }
Is this the correct behavior? Or should it return the removed element?
Sorry, I misread the code. The method does return the removed element.
In general it's still better to return from the method as early as possible:
private IHttpConnection getConnForOp(String op) { ...
if (op == null) { // return immediately return mConns.remove(mNumConns); }
// otherwise continue with the method IHttpConnection retConn = null; ... }