apache / apache/lucene

Code Optimizations in FieldType [LUCENE-8487]

Open
#9,533 1 comment 0 reactions 0 assignees View on GitHub
legacy-jira-label:optimization legacy-jira-priority:Minor module:core/other type:enhancement
Dominant language
Java
Stars
3.6k
Forks
1.4k
Avg merge
2d 11h
Merged PRs (30d)
88

Description

1) Delete unnecessary _if statement_.

```java
// Line 281 method
public void setDimensions(int dimensionCount, int dimensionNumBytes) {
...
if (dimensionCount == 0) {
if (dimensionNumBytes != 0) {
throw new IllegalArgumentException("when dimensionCount is 0, dimensionNumBytes must 0; got " + dimensionNumBytes);
}
} else if (dimensionNumBytes == 0) {
if (dimensionCount != 0) {
throw new IllegalArgumentException("when dimensionNumBytes is 0, dimensionCount must 0; got " + dimensionCount);
}
}
...
}
```

In this code, we can see that the following _if statement_ is unnecessary.

```java
if (dimensionCount != 0) {
throw new IllegalArgumentException("when dimensionNumBytes is 0, dimensionCount must 0; got " + dimensionCount);
}
```

Because it is a condition that is already processed in the upper _if statement_
(if dimensionCount == 0)

So I made the following code.

```java
// Line 281 method
public void setDimensions(int dimensionCount, int dimensionNumBytes) {
...
if (dimensionCount == 0) {
if (dimensionNumBytes != 0) {
throw new IllegalArgumentException("when dimensionCount is 0, dimensionNumBytes must 0; got " + dimensionNumBytes);
}
} else if (dimensionNumBytes == 0) {
throw new IllegalArgumentException("when dimensionNumBytes is 0, dimensionCount must 0; got " + dimensionCount);
}
...
}
```

2) Simplify _if statement_

```java
// Line 417 method
public boolean equals(Object obj) {
if (this == obj) return true;
if (obj == null) return false;
...
if (storeTermVectors != other.storeTermVectors) return false;
if (stored != other.stored) return false;
if (tokenized != other.tokenized) return false;
return true;
}
```

The final _if statement_ can be simplified.

```java
// Line 417 method
public boolean equals(Object obj) {
if (this == obj) return true;
if (obj == null) return false;
...
if (storeTermVectors != other.storeTermVectors) return false;
if (stored != other.stored) return false;
return tokenized == other.tokenized;
}
```

But I worry that change will hinder readability.
For that reason, if it is not good, I will not reflect it in the patch.

---
Migrated from [LUCENE-8487](https://issues.apache.org/jira/browse/LUCENE-8487) by Namgyu Kim (@danmuzi)
Attachments: [LUCENE-8487.patch](https://apache.github.io/lucene-jira-archive/attachments/LUCENE-8487/LUCENE-8487.patch)

Contributor guide

Open the contributing guide

Research direction

Start in FieldType.java at setDimensions and equals, using the method and line references in the issue. Review the attached LUCENE-8487.patch and confirm that the proposed conditional simplifications preserve behavior and readability; done means the agreed changes are applied without altering existing semantics.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
search
Issue type
Refactor
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.