[jira] Issue Comment Edited: (LUCENE-2784) Change all FilteredTermsEnum impls into TermsEnum decorators

["Uwe Schindler (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/LUCENE-2784?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12964837#action_12964837 ] 

Uwe Schindler edited comment on LUCENE-2784 at 11/29/10 12:13 PM:
------------------------------------------------------------------

bq. or a crazier idea, we could change MTQ.getTermsEnum(IndexReader reader, AttributeSource atts) to instead take a TermsEnum versus a reader.

I already checked for that (that was the reason for my prev. comment).

But in my opinion, we should not do this. As some MTQs may quicker return EMPTY for some reasons, like for a range when upper<lower. So you don't even need to look into the IR's fields/terms. So I would lie to remove the checks in MTQ and simply call always MTQ.getTermsEnum and then exit if empty. Thats much more clean. I was just wondering why these checks were added by Mike? For TermRange and Numeric its too much work.

      was (Author: thetaphi):
    bq. or a crazier idea, we could change MTQ.getTermsEnum(IndexReader reader, AttributeSource atts)
to instead take a TermsEnum versus a reader.

I already checked for that (that was the reason for my prev. comment).

But in my opinion, we should not do this. As some MTQs may quicker return EMPTY for some reasons, like for a range when upper<lower. So you don't even need to look into the IR's fields/terms. So I would lie to remove the checks in MTQ and simply call always MTQ.getTermsEnum and then exit if empty. Thats much more clean. I was just wondering why these checks were added by Mike? For TermRange and Numeric its too much work.
  
> Change all FilteredTermsEnum impls into TermsEnum decorators
> ------------------------------------------------------------
>
>                 Key: LUCENE-2784
>                 URL: https://issues.apache.org/jira/browse/LUCENE-2784
>             Project: Lucene - Java
>          Issue Type: Improvement
>            Reporter: Robert Muir
>            Assignee: Uwe Schindler
>             Fix For: 4.0
>
>         Attachments: LUCENE-2784.patch
>
>
> Currently, FilteredTermsEnum has two ctors:
> * FilteredTermsEnum(IndexReader reader, String field)
> * FilteredTermsEnum(TermsEnum tenum)
> But most of our concrete implementations (e.g. TermsRangeEnum) use the IndexReader+field ctor
> In my opinion we should remove this ctor, and switch over all FilteredTermsEnum implementations to just take a TermsEnum.
> Advantages:
> * This simplifies FilteredTermsEnum and its subclasses, where they are more decorator-like (perhaps in the future we could compose them)
> * Removes silly checks such as if (tenum == null) in every next()
> * Allows for consumers to pass in enumerators however they want: e.g. its their responsibility if they want to use MultiFields or not, it shouldnt be buried in FilteredTermsEnum.
> I created a quick patch (all core+contrib+solr tests pass), but i think this opens up more possibilities for refactoring improvements that haven't yet been done in the patch: we should explore these too.

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


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