hendrikmuhs commented on a change in pull request #460:
URL: https://github.com/apache/lucene/pull/460#discussion_r762011132



##########
File path: lucene/core/src/java/org/apache/lucene/util/fst/FST.java
##########
@@ -1000,6 +1027,98 @@ private void writePresenceBits(
     assert bytePos - dest == numPresenceBytes;
   }
 
+  private long estimateNodeAddress(

Review comment:
       Nevertheless I agree to you concern regarding code maintainability. This 
heuristic is in parts a copy of the logic later on.
   
   I will try to make improvements, maybe parts can re-used. Apart from that 
unit tests should be added, for the parts I can't de-dup, these tests can 
assert the correctness if one part is changed without adapting the other.

##########
File path: lucene/core/src/java/org/apache/lucene/util/fst/FST.java
##########
@@ -1000,6 +1027,98 @@ private void writePresenceBits(
     assert bytePos - dest == numPresenceBytes;
   }
 
+  private long estimateNodeAddress(

Review comment:
       Nevertheless I agree to your concern regarding code maintainability. 
This heuristic is in parts a copy of the logic later on.
   
   I will try to make improvements, maybe parts can re-used. Apart from that 
unit tests should be added, for the parts I can't de-dup, these tests can 
assert the correctness if one part is changed without adapting the other.




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