jpountz commented on code in PR #12040:
URL: https://github.com/apache/lucene/pull/12040#discussion_r1057563242
##########
lucene/core/src/java/org/apache/lucene/search/BooleanQuery.java:
##########
@@ -204,11 +204,11 @@ BooleanQuery rewriteNoScoring(IndexSearcher
indexSearcher) throws IOException {
for (BooleanClause clause : clauses) {
Query query = clause.getQuery();
Query rewritten = new ConstantScoreQuery(query).rewrite(indexSearcher);
- if (rewritten instanceof ConstantScoreQuery) {
- rewritten = ((ConstantScoreQuery) rewritten).getQuery();
+ if (rewritten instanceof ConstantScoreQuery constantScoreQuery) {
+ rewritten = constantScoreQuery.getQuery();
}
BooleanClause.Occur occur = clause.getOccur();
- if (occur == Occur.SHOULD && keepShould == false) {
+ if (occur == Occur.SHOULD && !keepShould) {
Review Comment:
we use `== false` intentionally across the codebase for better readability,
can you keep it and other places in this class too?
##########
lucene/core/src/java/org/apache/lucene/search/BooleanQuery.java:
##########
@@ -346,21 +338,15 @@ public Query rewrite(IndexSearcher indexSearcher) throws
IOException {
if (clauseSets.get(Occur.FILTER).size() > 0) {
final Set<Query> filters = new HashSet<>(clauseSets.get(Occur.FILTER));
boolean modified = false;
- if (filters.size() > 1 || clauseSets.get(Occur.MUST).isEmpty() == false)
{
+ if (filters.size() > 1 || !clauseSets.get(Occur.MUST).isEmpty()) {
modified = filters.remove(new MatchAllDocsQuery());
}
modified |= filters.removeAll(clauseSets.get(Occur.MUST));
if (modified) {
BooleanQuery.Builder builder = new BooleanQuery.Builder();
builder.setMinimumNumberShouldMatch(getMinimumNumberShouldMatch());
- for (BooleanClause clause : clauses) {
- if (clause.getOccur() != Occur.FILTER) {
- builder.add(clause);
- }
- }
- for (Query filter : filters) {
- builder.add(filter, Occur.FILTER);
- }
+ clauses.stream().filter(clause -> clause.getOccur() !=
Occur.FILTER).forEach(builder::add);
+ filters.forEach(filter -> builder.add(filter, Occur.FILTER));
Review Comment:
I think using streams above to compute `clauseCount` is fine as it makes the
code less verbose while remaining easy to read. I'm less sure about this
change: it's shorter, but it also reads harder for me.
##########
lucene/core/src/java/org/apache/lucene/search/BooleanQuery.java:
##########
@@ -408,19 +393,15 @@ public Query rewrite(IndexSearcher indexSearcher) throws
IOException {
if (shouldClauses.size() != clauseSets.get(Occur.SHOULD).size()) {
BooleanQuery.Builder builder =
new
BooleanQuery.Builder().setMinimumNumberShouldMatch(minimumNumberShouldMatch);
- for (Map.Entry<Query, Double> entry : shouldClauses.entrySet()) {
- Query query = entry.getKey();
- float boost = entry.getValue().floatValue();
- if (boost != 1f) {
- query = new BoostQuery(query, boost);
- }
- builder.add(query, Occur.SHOULD);
- }
- for (BooleanClause clause : clauses) {
- if (clause.getOccur() != Occur.SHOULD) {
- builder.add(clause);
- }
- }
+ shouldClauses.forEach(
+ (query, value) -> {
+ float boost = value.floatValue();
+ if (boost != 1f) {
+ query = new BoostQuery(query, boost);
+ }
+ builder.add(query, Occur.SHOULD);
+ });
+ clauses.stream().filter(clause -> clause.getOccur() !=
Occur.SHOULD).forEach(builder::add);
Review Comment:
Likewise here, streams don't make the code easier to read?
--
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.
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
