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

Yonik Seeley commented on LUCENE-2197:
--------------------------------------

Sorry Simon... I think I just got fed up with stuff like this in the JDK over 
the years (that forces people to write their own implementations for best 
performance), and you happened to be the closest person at the time :-)

Related: I'm the one who added this to BooleanQuery some time ago:
{code}
  /** Returns the list of clauses in this query. */
  public List<BooleanClause> clauses() { return clauses; }
{code}
Yes, it probably should also say something like "Don't modify - it may change 
the query" to the comments.
To the software pedant, that's not safe and would probably be called bad design 
- but I strongly believe that our API should be for adults, and one should be 
able to introspect objects like Queries w/o suffering object allocations.  We 
should also continue to develop Lucene for *ourselves*, not for some mythic 
stupid user... I've seen too many bad design decisions based on "this will 
confuse users" arguments rather than "this is confusing".

Sometimes it comes down to people trying to solve a class of problems that 
others aren't even having issues with - I don't ever recall someone 
accidentally modifying the set after they passed it to the StopFilter, or 
someone accidentally modifying clauses() from BooleanQuery.

I also disagree with checking all input parameters in many cases (things that 
could possibly be in someones inner loop and will throw an exception anyway).

Say we have this piece of code:
{code}
boolean checkLength(String str) {
  return str.length() < MY_MAX_LENGTH;
}
{code}

I think it's silly to add an explicit null check like so (but you see plenty of 
code like that):
{code}
boolean checkLength(String str) {
  if (str == null) {
     throw new RuntimeException("Can't pass checkLength a null string");
  }
  return str.length() < MY_MAX_LENGTH;
}
{code}


There.  Is that reply long enough for you ;-)

> StopFilter should not create a new CharArraySet if the given set is already 
> an instance of CharArraySet
> -------------------------------------------------------------------------------------------------------
>
>                 Key: LUCENE-2197
>                 URL: https://issues.apache.org/jira/browse/LUCENE-2197
>             Project: Lucene - Java
>          Issue Type: Bug
>          Components: Analysis
>    Affects Versions: 3.1
>            Reporter: Simon Willnauer
>            Priority: Critical
>             Fix For: 3.1
>
>         Attachments: LUCENE-2197.patch, LUCENE-2197.patch
>
>
> With LUCENE-2094 a new CharArraySet is created no matter what type of set is 
> passed to StopFilter. This does not behave as  documented and could introduce 
> serious performance problems. Yet, according to the javadoc, the instance of 
> CharArraySet should be passed to CharArraySet.copy (which is very fast for 
> CharArraySet instances) instead of "copied" via "new CharArraySet()"

-- 
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: java-dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: java-dev-h...@lucene.apache.org

Reply via email to