[Pki-devel] [PATCH] 862 HttpConnFactory addendum

[Christina Fu]

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;
>        ...
>    }
>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-trac-ticket-862-HTTP-connection-factory-multi-uri-ad.patch
Type: text/x-patch
Size: 14961 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/pki-devel/attachments/20140324/8b8a8396/attachment.bin>