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

Kay Kay commented on SOLR-916:
------------------------------

| I could go either way. The name change certainly makes some sense, but even 
as is, a core that is returned closed is not very useful. Null could just as 
well be returned. But when you ask to returnPrev, a working open core is 
guaranteed to be returned if a previous one existed. I wouldnt feel so bad 
leaving it as it is - your change almost seems to strengthen the contract - you 
have to return the closed core as well. Neither side is a very strong argument 
to me and I could go either way.

I do not have an issue with the logic ( closing an existing core if returnPrev 
is false ) as it is currently but am more concerned with the name of the 
variable and the documentation about the same - just to make sure that it is in 
sync with the actual logic as of today. 

> CoreContainer :: register(String, SolrCore, boolean)  documentation 
> clarification about returnPrev argument
> -----------------------------------------------------------------------------------------------------------
>
>                 Key: SOLR-916
>                 URL: https://issues.apache.org/jira/browse/SOLR-916
>             Project: Solr
>          Issue Type: Improvement
>    Affects Versions: 1.3
>         Environment: Tomcat 6, JRE 6 
>            Reporter: Kay Kay
>            Priority: Minor
>             Fix For: 1.4
>
>         Attachments: SOLR-916.patch
>
>   Original Estimate: 2h
>  Remaining Estimate: 2h
>
> In CoreContainer.java :: register(name, core, returnPrev) - the documentation 
> says 
>   it would return a previous core having the same name if it existed *and 
> returnPrev = true*.
>   * @return a previous core having the same name if it existed and 
> returnPrev==true
>   */
>  public SolrCore register(String name, SolrCore core, boolean returnPrev) ..
> But as per the code towards the end - the previous core is returned anyway, 
> irrespective of the value of returnPrev. The difference, though, seems to be 
> that when returnPrev is false, the previous core (of the same name, if 
> exists) is closed.
> Which one of them is correct . If the code were correct , would the variable 
> be better renamed as closePrevious , as opposed to returnPrevious.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply via email to