Re: [Pki-devel] [PATCH] 862 HttpConnFactory addendum
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;
...
}
Attachments:
- 0001-trac-ticket-862-HTTP-connection-factory-multi-uri-ad.patch (text/x-patch — 14.6 KB)