[GitHub] s1monw commented on a change in pull request #578: LUCENE-8699: Use fixed byte array in HeapPointWriter

[GitBox <gi...@apache.org>]

s1monw commented on a change in pull request #578: LUCENE-8699: Use fixed byte array in HeapPointWriter
URL: https://github.com/apache/lucene-solr/pull/578#discussion_r258002172
 
 

 ##########
 File path: lucene/core/src/java/org/apache/lucene/util/bkd/PointWriter.java
 ##########
 @@ -32,9 +32,12 @@
   /** Add a new point from byte array*/
   void append(byte[] packedValue, int docID) throws IOException;
 
-  /** Add a new point from byteRef */
+  /** Add a new point from byteRef and docId */
 
 Review comment:
   I don't like that this interface has 3 different append methods. I think it should have at most 2. From what I can tell we can easily get rid of `void append(byte[] packedValue, int docID)` and use a _scratch_ value where necessary. 
   
   the `void append(BytesRef packedValueWithDocId) throws IOException;` method is difficult since it expects the format to be correct. I would feel much better if we had a way to verify that the format is correct like passing the reader instead of a bytesRef or a dedicated interface. 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

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