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 and remove all null-check, while 
still throwing meaningful exception when users try to use a nkn-readable FST.
   - In the next PR, `NullFSTReader` can be used for the temporary FST created 
during building so it won't be throw-way code.



-- 
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

Reply via email to