uschindler commented on code in PR #12295:
URL: https://github.com/apache/lucene/pull/12295#discussion_r1391562831
##########
lucene/core/src/java/org/apache/lucene/codecs/lucene90/Lucene90PointsWriter.java:
##########
@@ -182,7 +181,7 @@ public void merge(MergeState mergeState) throws IOException
{
* a bulk merge of the points.
*/
for (PointsReader reader : mergeState.pointsReaders) {
- if (reader instanceof Lucene90PointsReader == false) {
+ if (!(reader instanceof Lucene90PointsReader)) {
Review Comment:
This change is not needed. Some people here prefer the `== false` so let's
keep it at least here.
##########
lucene/codecs/src/java/org/apache/lucene/codecs/memory/DirectPostingsFormat.java:
##########
@@ -1470,20 +1475,26 @@ public PostingsEnum postings(PostingsEnum reuse, int
flags) {
// TODO: the logic of which enum impl to choose should be refactored
to be simpler...
if (hasPos && PostingsEnum.featureRequested(flags,
PostingsEnum.POSITIONS)) {
- if (terms[termOrd] instanceof LowFreqTerm) {
- final LowFreqTerm term = ((LowFreqTerm) terms[termOrd]);
- final int[] postings = term.postings;
- final byte[] payloads = term.payloads;
+ var term = terms[termOrd];
Review Comment:
I would revert this whole cleanup here. The code was much more readable
before. The new syntax is fine if we only have one branch using the auto-casted
instance. Here we have if/else now using different syntax, so please revert!
##########
lucene/core/src/java/org/apache/lucene/util/RamUsageEstimator.java:
##########
@@ -448,38 +448,38 @@ private static long sizeOfObject(Object o, int depth,
long defSize) {
return 0;
}
long size;
- if (o instanceof Accountable) {
- size = ((Accountable) o).ramBytesUsed();
- } else if (o instanceof String) {
- size = sizeOf((String) o);
- } else if (o instanceof boolean[]) {
- size = sizeOf((boolean[]) o);
- } else if (o instanceof byte[]) {
- size = sizeOf((byte[]) o);
- } else if (o instanceof char[]) {
- size = sizeOf((char[]) o);
- } else if (o instanceof double[]) {
- size = sizeOf((double[]) o);
- } else if (o instanceof float[]) {
- size = sizeOf((float[]) o);
- } else if (o instanceof int[]) {
- size = sizeOf((int[]) o);
- } else if (o instanceof Integer) {
- size = sizeOf((Integer) o);
- } else if (o instanceof Long) {
- size = sizeOf((Long) o);
- } else if (o instanceof long[]) {
- size = sizeOf((long[]) o);
- } else if (o instanceof short[]) {
- size = sizeOf((short[]) o);
- } else if (o instanceof String[]) {
- size = sizeOf((String[]) o);
- } else if (o instanceof Query) {
- size = sizeOf((Query) o, defSize);
- } else if (o instanceof Map) {
- size = sizeOfMap((Map<?, ?>) o, ++depth, defSize);
- } else if (o instanceof Collection) {
- size = sizeOfCollection((Collection<?>) o, ++depth, defSize);
+ if (o instanceof Accountable accountable) {
Review Comment:
Rewrite this to pattern-matching (new) switch statement.
##########
lucene/codecs/src/java/org/apache/lucene/codecs/memory/DirectPostingsFormat.java:
##########
@@ -1470,20 +1475,26 @@ public PostingsEnum postings(PostingsEnum reuse, int
flags) {
// TODO: the logic of which enum impl to choose should be refactored
to be simpler...
if (hasPos && PostingsEnum.featureRequested(flags,
PostingsEnum.POSITIONS)) {
- if (terms[termOrd] instanceof LowFreqTerm) {
- final LowFreqTerm term = ((LowFreqTerm) terms[termOrd]);
- final int[] postings = term.postings;
- final byte[] payloads = term.payloads;
+ var term = terms[termOrd];
+ if (term instanceof LowFreqTerm) {
+ final LowFreqTerm lowFreqTerm = (LowFreqTerm) terms[termOrd];
+ final int[] postings = lowFreqTerm.postings;
+ final byte[] payloads = lowFreqTerm.payloads;
return new LowFreqPostingsEnum(hasOffsets,
hasPayloads).reset(postings, payloads);
} else {
- final HighFreqTerm term = (HighFreqTerm) terms[termOrd];
+ final HighFreqTerm highFreqTerm = (HighFreqTerm) terms[termOrd];
return new HighFreqPostingsEnum(hasOffsets)
- .reset(term.docIDs, term.freqs, term.positions, term.payloads);
+ .reset(
+ highFreqTerm.docIDs,
+ highFreqTerm.freqs,
+ highFreqTerm.positions,
+ highFreqTerm.payloads);
}
}
- if (terms[termOrd] instanceof LowFreqTerm) {
- final int[] postings = ((LowFreqTerm) terms[termOrd]).postings;
+ var term = terms[termOrd];
Review Comment:
same here.
##########
lucene/core/src/java/org/apache/lucene/util/IOUtils.java:
##########
@@ -390,16 +390,16 @@ public static Error rethrowAlways(Throwable th) throws
IOException, RuntimeExcep
throw new AssertionError("rethrow argument must not be null.");
}
- if (th instanceof IOException) {
- throw (IOException) th;
+ if (th instanceof IOException ioException) {
Review Comment:
Here we could also use pattern-matching (new) switch.
##########
lucene/highlighter/src/java/org/apache/lucene/search/highlight/WeightedSpanTermExtractor.java:
##########
@@ -607,23 +604,22 @@ protected void collectSpanQueryFields(SpanQuery
spanQuery, Set<String> fieldName
protected boolean mustRewriteQuery(SpanQuery spanQuery) {
if (!expandMultiTermQuery) {
return false; // Will throw UnsupportedOperationException in case of a
SpanRegexQuery.
- } else if (spanQuery instanceof FieldMaskingSpanQuery) {
Review Comment:
and here.
##########
lucene/core/src/java/org/apache/lucene/util/bkd/OfflinePointReader.java:
##########
@@ -144,10 +144,10 @@ public PointValue pointValue() {
@Override
public void close() throws IOException {
try {
- if (countLeft == 0 && in instanceof ChecksumIndexInput && checked ==
false) {
+ if (countLeft == 0 && in instanceof ChecksumIndexInput checksumIn &&
!checked) {
Review Comment:
does the `== false` work if we reorder the clauses and move `checked ==
false` before the instanceof check?
##########
lucene/core/src/java/org/apache/lucene/index/SoftDeletesDirectoryReaderWrapper.java:
##########
@@ -69,13 +69,13 @@ protected DirectoryReader
doWrapDirectoryReader(DirectoryReader in) throws IOExc
Map<CacheKey, LeafReader> readerCache = new HashMap<>();
for (LeafReader reader : getSequentialSubReaders()) {
// we try to reuse the life docs instances here if the reader cache key
didn't change
- if (reader instanceof SoftDeletesFilterLeafReader &&
reader.getReaderCacheHelper() != null) {
+ if (reader instanceof SoftDeletesFilterLeafReader
softDeletesFilterLeafReader
Review Comment:
here we could use a pattern matching (new) switch statement!
##########
lucene/analysis/morfologik/src/java/org/apache/lucene/analysis/morfologik/MorphosyntacticTagsAttributeImpl.java:
##########
@@ -52,8 +52,8 @@ public void clear() {
@Override
public boolean equals(Object other) {
- if (other instanceof MorphosyntacticTagsAttribute) {
- return equal(this.getTags(), ((MorphosyntacticTagsAttribute)
other).getTags());
+ if (other instanceof MorphosyntacticTagsAttribute
morphosyntacticTagsAttribute) {
Review Comment:
Yes, but it should be cleaned up later when we standardize equals/hashCode.
##########
lucene/highlighter/src/java/org/apache/lucene/search/highlight/WeightedSpanTermExtractor.java:
##########
@@ -127,17 +127,15 @@ public WeightedSpanTermExtractor(String defaultField) {
*/
protected void extract(Query query, float boost, Map<String,
WeightedSpanTerm> terms)
throws IOException {
- if (query instanceof BoostQuery) {
- BoostQuery boostQuery = (BoostQuery) query;
+ if (query instanceof BoostQuery boostQuery) {
Review Comment:
rewrite the whole thing as a pattern-matching (new) switch statement.
##########
lucene/highlighter/src/java/org/apache/lucene/search/highlight/WeightedSpanTermExtractor.java:
##########
@@ -585,18 +582,18 @@ public Map<String, WeightedSpanTerm>
getWeightedSpanTermsWithScores(
}
protected void collectSpanQueryFields(SpanQuery spanQuery, Set<String>
fieldNames) {
- if (spanQuery instanceof FieldMaskingSpanQuery) {
- collectSpanQueryFields(((FieldMaskingSpanQuery)
spanQuery).getMaskedQuery(), fieldNames);
- } else if (spanQuery instanceof SpanFirstQuery) {
- collectSpanQueryFields(((SpanFirstQuery) spanQuery).getMatch(),
fieldNames);
- } else if (spanQuery instanceof SpanNearQuery) {
- for (final SpanQuery clause : ((SpanNearQuery) spanQuery).getClauses()) {
+ if (spanQuery instanceof FieldMaskingSpanQuery fieldMaskingSpanQuery) {
Review Comment:
same here.
--
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
