msokolov commented on a change in pull request #767: LUCENE-8904: enhance Nori 
DictionaryBuilder tool
URL: https://github.com/apache/lucene-solr/pull/767#discussion_r301200364
 
 

 ##########
 File path: 
lucene/analysis/nori/src/tools/java/org/apache/lucene/analysis/ko/util/BinaryDictionaryWriter.java
 ##########
 @@ -137,14 +139,17 @@ public int put(String[] entry) {
       flags |= BinaryDictionary.HAS_READING;
     }
 
-    assert leftId < 8192; // there are still unused bits
-    assert posType.ordinal() < 4;
+    if (leftId >= ID_LIMIT) {
+      throw new IllegalArgumentException("leftId >= " + ID_LIMIT + ": " + 
leftId);
+    }
+    if (posType.ordinal() >= 4) {
+      throw new IllegalArgumentException("posType.ordinal() >= " + 4 + ": " + 
posType.ordinal());
+    }
     buffer.putShort((short)(leftId << 2 | posType.ordinal()));
     buffer.putShort((short) (rightId << 2 | flags));
     buffer.putShort(wordCost);
 
     if (posType == POS.Type.MORPHEME) {
-      assert leftPOS == rightPOS;
 
 Review comment:
   Well, assertions are always unnecessary! I think their value is exactly in 
helping ensure that the correct code path was followed to get here, and 
confirming what we know to be true by analysis or anyway knowledge, in fact 
holds in practice (empirically). For example, if I were to somehow mess up the 
code above that guarantees this, due to my ignorance, this assertion would 
helpfully let me know. So I think we should keep it?  

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org

Reply via email to