[GitHub] [lucene] gautamworah96 commented on pull request #12427: StringsToAutomaton#build to take List as parameter instead of Collection

["gautamworah96 (via GitHub)" <gi...@apache.org>]

gautamworah96 commented on PR #12427:
URL: https://github.com/apache/lucene/pull/12427#issuecomment-1631629455

   I was looking at this problem too and the change in test classes to meld arbitrary collections to `Lists` irked me as well. 
    
   +1 to throw an `IllegalArgumentException` in `#add` instead of having the user debug the error later on. 
   
   On a related note, since we only traverse the items in their given order, maybe we can relax `StringsToAutomaton#build(Collection<BytesRef>, boolean)` to `StringsToAutomaton#build(Iterable<BytesRef> input, boolean)`. This change will also make it more consistent with the `build(BytesRefIterator input, boolean asBinary)` method.


-- 
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-unsubscribe@lucene.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


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