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