dungba88 commented on code in PR #12624:
URL: https://github.com/apache/lucene/pull/12624#discussion_r1402058230
##########
lucene/core/src/java/org/apache/lucene/util/fst/FST.java:
##########
@@ -435,6 +433,13 @@ public FST(FSTMetadata<T> metadata, DataInput in,
Outputs<T> outputs, FSTStore f
this.fstReader = fstReader;
}
+ /**
+ * @return true if and only if this FST is readable (e.g has a reverse
BytesReader)
+ */
+ public boolean isReadable() {
Review Comment:
Yeah, I was thinking as we will remove it anyway, maybe we don't need this,
along with the null check (as they seems to be throw-away code). If users try
to use a non-readable FST now, NPE will thrown instead (of
IllegalStateExeption). And then when we change FSTCompiler to return
FSTMetadata, they can't even create a non-readable FST. What do you think?
Alternatively, we can double back to `NullFSTReader`, but let it throw
exception like this:
```
private static final class NullFSTReader implements FSTReader {
@Override
public long ramBytesUsed() {
return 0;
}
@Override
public FST.BytesReader getReverseBytesReader() {
throw new UnsupportedOperationException("NullFSTReader does not
support getReverseBytesReader()");
}
@Override
public void writeTo(DataOutput out) {
throw new UnsupportedOperationException("NullFSTReader does not
support writeTo(DataOutput)");
}
}
```
The benefits are:
- We can enforce `FSTReader` to be non-null.
- Remove all null-check.
In the next PR, `NullFSTReader` can be used for the temporary FST created
during building.
--
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
