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

Robert Muir commented on LUCENE-6033:
-------------------------------------

{quote}
I think it's easier to make the case that CachingTokenFilter should have been 
propagating reset() from it's fillCache() all along, and thus you would then 
use CachingTokenFilter in a more normal way – wrap it and call reset() then 
increment in a loop, etc., instead of knowing you need to reset() on what it 
wraps but not this token filter itself. That's weird. It's ab-normal for a 
TokenFilter to never propagate reset, so every user of CachingTokenFilter to 
date has worked around this by calling reset() on the underlying input instead 
of the final wrapping token filter (CachingTokenFilter in this case). To be 
clear, CachingTokenFilter.reset() didn't and still doesn't with this patch 
propagate reset(), it happens the one time it consumes the stream indirectly 
via incrementToken().
{quote}

I mostly agree with this. Except i think this filter should propogate reset() 
from its own reset(), only once (then once cached, it obviously doesnt 
propogate any such call).

But then why propose an option? I would rather fix the behavior of the this 
thing and not have "two ways to do it". Consumers of the TS api already have a 
hard enough time. 

We can do a break, since its 5.0, and there arent many consumers of 
cachingtokenfilter. But one of them is a big one, queryparser, which is not 
good. Another option is to deprecate it and just make a new one beside it with 
a different name and the new "good" reset behavior.

> Add CachingTokenFilter.isCached and switch LinkedList to ArrayList
> ------------------------------------------------------------------
>
>                 Key: LUCENE-6033
>                 URL: https://issues.apache.org/jira/browse/LUCENE-6033
>             Project: Lucene - Core
>          Issue Type: Improvement
>            Reporter: David Smiley
>            Assignee: David Smiley
>             Fix For: 5.0, Trunk
>
>         Attachments: LUCENE-6033.patch, 
> LUCENE-6033_boolean_resetInput_option.patch
>
>
> CachingTokenFilter could use a simple boolean isCached() method implemented 
> as-such:
> {code:java}
>   /** If the underlying token stream was consumed and cached */
>   public boolean isCached() {
>     return cache != null;
>   }
> {code}
> It's useful for the highlighting code to remove its wrapping of 
> CachingTokenFilter if after handing-off to parts of its framework it turns 
> out that it wasn't used.
> Furthermore, use an ArrayList, not a LinkedList.  ArrayList is leaner when 
> the token count is high, and this class doesn't manipulate the list in a way 
> that might favor LL.
> A separate patch will come that actually uses this method.



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

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to