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

Chris A. Mattmann commented on SOLR-1688:
-----------------------------------------

{quote}
Rhetorical question: should all implementations of Map be in the java.util 
package? 
{quote}

If they are defined as a "core" data structure part of the JDK, then I would 
say yes. It's not as black and white of a line as you make it out to be. You 
can have SOLR be entirely a plugin-based system, with nothing but configuration 
inside of SVN, or you can have every piece of code that interacts with SOLR be 
inside the SOLR SVN. Neither solution will work, you have to strike a balance. 
The same applies for code organization and using absolutes or extremes doesn't 
really illustrate much.

{quote}
Think of it this way - some FieldType's may implement their getValueSource with 
a publicly defined ValueSource and others may not. 
{quote}

This makes sense so long as there is a reason. Can you tell me the reason that 
e.g., StrFieldSource exists inside of StrField while  DoubleFieldSource exists 
outside of DoubleField? Or why the other 4 or 5 FieldSources that are defined 
inside of their own java file exist there, while the other 4 or 5 defined 
inside of the FieldType's java file exist there? What's the litmus test? I'm 
not sure I see one, which is why I created this issue. And let's be clear. 
Consistency, code style, maintainability _are_ important issues and following a 
pattern in one situation while not following it in another (similar or same) 
situation is not a great example of style or consistency.

{quote}It's not the case that for a given FooFieldType that there should be a 
publicly usable FooValueSource. There should never be any guarantee that a 
specific FieldType use a specific implementation of a ValueSource. The only 
guarantee should be that it work. {quote}

The patch I put up still allows everything to work and doesn't change this 
requirement.

{quote}
And if that's the case, one should ask why all these value sources should be 
public?
{quote}

Because it's more consistent, and thus, more maintainable. Because when you 
tell someone to modify one of the core FieldSources or ValueSources, they know 
where to look instead of, "oh is this one inside of a class inside of 
o.a.solr.schema, or is this one in the o.a.solr.search.function package"? You 
can argue that it doesn't take much time to make that determination, which I'll 
give you, but I'll argue back it isn't consistent. If there's not a good reason 
to make them all live in search.function or live inside of the FieldTypes in 
schema, and it's really a case-by-case reason, then I'd like to hear some of 
those case-by-cases. 


> Inner class FieldCacheSources should be refactored into their own classes
> -------------------------------------------------------------------------
>
>                 Key: SOLR-1688
>                 URL: https://issues.apache.org/jira/browse/SOLR-1688
>             Project: Solr
>          Issue Type: Improvement
>          Components: search
>    Affects Versions: 1.4
>         Environment: indep. of env.
>            Reporter: Chris A. Mattmann
>             Fix For: 1.5
>
>         Attachments: SOLR-1688.Mattmann.122609.patch.txt
>
>
> While working on SOLR-1586 I noticed that outside of class level access (or 
> package level), you can't really reference FieldCacheSources that are defined 
> inside of their FieldType constituents (e.g., in the case of StrFieldSource 
> as defined in StrField). What's more troubling is that the 
> FieldType/FieldCacheSources are defined in an inconsistent fashion: some are 
> done as inner classes, e.g., StrFieldSource and SortableFloatFieldSource, 
> while others are defined as individual classes (e.g., FloatFIeldSource). This 
> patch will make them all consistent and define each FieldCacheSource as an 
> outside class, present in o.a.solr.search.function.

-- 
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