lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Shai Erera (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (SOLR-8475) Some refactoring to SolrIndexSearcher
Date Sat, 02 Jan 2016 19:52:39 GMT

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

Shai Erera commented on SOLR-8475:
----------------------------------

bq. Once something is committed I just tend to leave the formatting alone unless I'm actively
editing that part of the code, or if it's really bad.

I usually follow that pattern too, however in this case I also re-formatted {{SolrIndexSearcher}}
code (thought it'd be nice). But also, if we don't change our standard settings, then with
the current settings, whenever I'll hit ENTER in a class, it will indent the empty line. Or,
if I write new code and want to use that builder-pattern, it will rewrap lines. So, I think
we should change those two settings that I included in the patch. I doubt that people really
want to see indented empty lines, or forcing re-wrapping of wrapped lines.

bq. it's usually only constants (i.e. static final strings/ints/empty_maps)

LOG is a static final field and I try to follow that convention with any static-final field.
Also, we already sometimes use "log" and other times "logger" and even with the casing we're
not consistent ({{LineRandomizedMapper}}, {{TreeMergeMapper}}, {{SafeConcurrentUpdateSolrClient}}
to name a few).

bq. Back compat breaks

True, but it's a really simple one to address ("Organize Imports" in any modern IDE). I can
add a note in CHANGES? Really, the class is huge and even though extracting those two classes
does not greatly reduce its size, it's a change in the right direction. And it's really a
simple break to address. What do you think?

> Some refactoring to SolrIndexSearcher
> -------------------------------------
>
>                 Key: SOLR-8475
>                 URL: https://issues.apache.org/jira/browse/SOLR-8475
>             Project: Solr
>          Issue Type: Improvement
>          Components: search
>            Reporter: Shai Erera
>            Assignee: Shai Erera
>            Priority: Minor
>             Fix For: 5.5, Trunk
>
>         Attachments: SOLR-8475.patch, SOLR-8475.patch, SOLR-8475.patch, SOLR-8475.patch
>
>
> While reviewing {{SolrIndexSearcher}}, I started to correct a thing here and there, and
eventually it led to these changes:
> * Moving {{QueryCommand}} and {{QueryResult}} to their own classes.
> * Moving FilterImpl into a private static class (was package-private and defined in the
same .java file, but separate class).
> * Some code formatting, imports organizing and minor log changes.
> * Removed fieldNames (handled the TODO in the code)
> * Got rid of usage of deprecated classes such as {{LegacyNumericUtils}} and {{Legacy-*-Field}}.
> I wish we'd cut down the size of this file much more (it's 2500 lines now), but I've
decided to stop here so that the patch is manageable. I would like to explore further refactorings
afterwards, e.g. extracting cache management code to an outer class (but keep {{SolrIndexSearcher}}'s
API the same, if possible).
> If you have additional ideas of more cleanups / simplifications, I'd be glad to do them.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


Mime
View raw message