lucene-solr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Chris A. Mattmann (JIRA)" <j...@apache.org>
Subject [jira] Commented: (SOLR-1688) Inner class FieldCacheSources should be refactored into their own classes
Date Sun, 27 Dec 2009 22:30:30 GMT

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


Mime
View raw message