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

Shawn Heisey commented on SOLR-10915:
-------------------------------------

If I'm understanding what's going on here, I really like it.  I hadn't thought 
of using the builder in the constructor ... that's a really good idea.

It sounds like this would make both the client itself and its builder 
user-extendable.

I did notice a couple of things in the patch, not sure if these are actual 
problems or a lack of understanding on my part:

 * Where a Builder class has been added, it doesn't have any explicit 
visibility, while the existing Builder classes are explicitly marked public.
 * The new Builder classes do not have any fluent "with*" methods, only a 
multiple-argument constructor.  The existing ones have simple constructors 
(no-arg and in some cases one-arg) and fluent methods for configuration.

As for how to handle removal and deprecation ... are we at a point where we can 
simply remove the other constructors in master/branch_7x?  They should have 
already been deprecated when the Builder pattern was implemented.  The 
remaining non-deprecated constructor (before this patch) is likely private or 
protected, and I don't think we have any reason to worry about deprecating it 
before we change it to one that accepts a Builder.


> Make SolrClient classes extendable without code duplication
> -----------------------------------------------------------
>
>                 Key: SOLR-10915
>                 URL: https://issues.apache.org/jira/browse/SOLR-10915
>             Project: Solr
>          Issue Type: Bug
>      Security Level: Public(Default Security Level. Issues are Public) 
>          Components: clients - java
>            Reporter: Anshum Gupta
>            Assignee: Anshum Gupta
>            Priority: Blocker
>             Fix For: master (7.0)
>
>         Attachments: SOLR-10915.patch.with-deprecations, 
> SOLR-10915.patch.without-deprecations
>
>
> SolrClient used to be easily extendable but our move to Builder pattern has 
> made it impossible to extend those classes without code duplication. 
> As an example, if we wanted to extend HttpSolrClient we would also want to 
> extend the Builder. The problem is that the constructor of the main class, 
> does not accept the builder object but explicit parameters, and the inherited 
> class doesn't have access to any of those values from the Builder object as 
> they are all private. 
> Generally, we'd want to have the client object constructor accept the 
> Builder, instead of explicit values, and also make the Builder values 
> protected so the inherited classes could use them.
> I'm marking this as a blocker for 7.0 as this is an important piece that 
> needs to be fixed before the major release, specially now that we know that 
> this problem exists.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

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

Reply via email to