micpalmia commented on issue #1343: LUCENE-8103: Use two-phase iteration in Query- and DoubleValuesSource URL: https://github.com/apache/lucene-solr/pull/1343#issuecomment-597801420 Copying comments by @dsmiley here: > minor: I suggest using the variable name "disi" for a DocIdSetIterator as this choice is quite common elsewhere. > > In DoubleValueSource I think it'd be better to declare the state you need like tpi, disi inside the anonymous DoubleValues class to keep the scope smaller and to reduce extra hidden indirection that I think Java adds. Also declare final where applicable. scorerMatch boolean is confusing to me; and I think the initial value as written doesn't matter either. I suggest renaming this to thisDocMatches, initialize to false (since we are unpositioned at the start), and then update it appropriately. The logic you have now around scoreMatch seems to do maybe extra work and/or isn't as simple as it could be. If doubleValue is invoked and not thisDocMatches then don't delegate to the scorer, which is probably invalid. Return NaN. > > In QueryValueSource.exists, maybe you could improve this a bit to be simpler; I find the way it was written prior to be confusing.
---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
