NightOwl888 commented on issue #763: URL: https://github.com/apache/lucenenet/issues/763#issuecomment-1322829882
> Here I did run into some classes which raises a question from a efficiency perspective. E.g. InputStreamDataInput that takes a Stream in it's constructor and immediately wraps that stream in a BinaryReader but I can't see for what reason. If the concern is that the underlying stream is not buffered, then perhaps a BufferedStream would be more appropriate. The Java version takes a InputStream in the constructor and just uses that as is, so I am not sure why this differs. (At least for the Java version I currently have checked out) Looks like you have spotted yet another weird thing that was done during the initial port. In Lucene, the passed-in stream was used without wrapping it in anything. It was changed like this in https://github.com/apache/lucenenet/commit/3bcd980f0244af667a885eb14e6144510af591e0#diff-5cbde6cbc3ee23bdc1d699d185ccae736530a7129bb2f2a844be24155fa9e9cd, and I don't see any valid reason for changing it like this. Care to submit a patch? Also, a null guard clause can be added to the constructor because clearly this class cannot function with a null stream. -- 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: dev-unsubscr...@lucenenet.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
