[ 
https://issues.apache.org/jira/browse/HTTPCLIENT-1238?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13500986#comment-13500986
 ] 

Sebb commented on HTTPCLIENT-1238:
----------------------------------

A few comments from looking at the code in isolation:

public static ClientConnectionManagerTracker instance; // should be private??
...
public static ClientConnectionManagerTracker getTracker()

Multiple threads can (re)create the tracker, so it won't be a singleton.
If that does not matter, it should be documented.
The field is public so can be updated independently - mutable static fields are 
inherently not thread-safe (in fact they are generally thread-hostile).

If it is intended to be a singleton, the code should sync the getTracker() 
method, or use an IODH.

The same remarks apply to ConfigurableProxySelector.

The ProxyConfiguration interface needs Javadoc for the methods.
The failed() method in particular needs to be carefully documented as it has 
side effects.

Javadoc elsewhere is also lacking.

TransparentProxySelector.FAILURES should be final.

PropertiesUtil  has @since 2.1, which looks wrong.

Not clear why the code throws UndeclaredThrowableException; this should be 
documented.

Sorry, cannot comment on the code in relation to HttpComponents itself.

                
> Contribute Bundle Activator And Central Proxy Configuration
> -----------------------------------------------------------
>
>                 Key: HTTPCLIENT-1238
>                 URL: https://issues.apache.org/jira/browse/HTTPCLIENT-1238
>             Project: HttpComponents HttpClient
>          Issue Type: Wish
>          Components: HttpClient
>    Affects Versions: 4.2.1
>            Reporter: Dominique Jäggi
>            Priority: Minor
>             Fix For: 4.3 Final
>
>         Attachments: HTTPCLIENT-1238-2.patch, HTTPCLIENT-1238-3.patch, 
> HTTPCLIENT-1238.patch
>
>
> as discussed at [0] i'd like to contribute the bundle activator and central 
> proxy configuration.
> the attached patch may need some cleanup on your side, as only assumed 
> locations where to put some classes or in which pom.xml to put dependencies.
> i kindly ask you to review the patch in if possible integrate it in a future 
> release.
> Adobe (and i as its employee) is available for assistance, explanations, or 
> further dev work required in the context of the patch.
> [0] 
> http://mail-archives.apache.org/mod_mbox/hc-dev/201209.mbox/%3cCACMijv-21S4+Jw_A=jdfhevb9cmt4kni5p3jahzfh3kvu4b...@mail.gmail.com%3e

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to