[
https://issues.apache.org/jira/browse/SOLR-1688?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12794782#action_12794782
]
Shalin Shekhar Mangar commented on SOLR-1688:
---------------------------------------------
{quote}
IMO, most of these should remain implementation details (i.e. not public)...
they weren't thought out in sufficient detail to support as public classes (and
there has been little reason to do so).
If we need StrValueSource to be public for another issue, then we should limit
the change to that.
{quote}
+1
As they say, lets not fix what ain't broken.
{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}
Chris, we are striving for balance and we are OK with the change to
StrFieldSource. In this particular case, you seem to be pushing towards
extremes in the name of consistency.
{quote}
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?
{quote}
It is not a public API and I guess that at the time it was written, there was
no reason to make it one. It was convenient or a matter of personal style or
most likely a random choice. There is no litmus test and there does not have to
be one.
{quote}
Because it's more consistent, and thus, more maintainable.
{quote}
Actually it is the other way round. Once you make it public, it is harder to
maintain. All changes should then be backward compatible as far as possible.
The bottom line is that making all of them public is not needed. Your opinion
is that it is broken because it is not consistent. My opinion is that it is OK
and it does not matter. We shouldn't lean towards making something a public API
in the name of consistency.
{quote}
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"?
{quote}
Most IDEs have a way to goto the source of a particular class, otherwise there
is grep. The point is that many (most?) of these classes don't need to be
modified unless in very rare cases. If it becomes a common practice to modify
them, then there is probably something wrong with our APIs and we need to
re-think them.
> 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.
