paulirwin commented on code in PR #1074: URL: https://github.com/apache/lucenenet/pull/1074#discussion_r1897068766
########## src/Lucene.Net.Analysis.Kuromoji/Dict/BinaryDictionary.cs: ########## @@ -101,8 +101,8 @@ protected BinaryDictionary() ByteBuffer buffer; // LUCENENET: IDE0059: Remove unnecessary value assignment using (Stream mapIS = GetResource(TARGETMAP_FILENAME_SUFFIX)) + using (var @in = new InputStreamDataInput(mapIS)) // LUCENENET: CA2000: Use using statement Review Comment: While I don't disagree with having leaveOpen versions, it should be noted that there's nothing wrong with calling dispose on the stream more than once. IDisposable guidance[1] is that Dispose should be able to be called more than once without issue, and the .NET BCL types are very good about ensuring this is true. I tested this with FileStream and StreamReader/Writer without issue before submitting this PR. Compared to the performance of i.e. writing to a file, an extra Dispose call is negligible performance-wise as well. That said, I don't see a good reason _not_ to go ahead and do these changes, so I'll knock them out. 1: https://learn.microsoft.com/en-us/dotnet/standard/garbage-collection/implementing-dispose > To help ensure that resources are always cleaned up appropriately, a [Dispose](https://learn.microsoft.com/en-us/dotnet/api/system.idisposable.dispose) method should be idempotent, such that it's callable multiple times without throwing an exception. Furthermore, subsequent invocations of [Dispose](https://learn.microsoft.com/en-us/dotnet/api/system.idisposable.dispose) should do nothing. -- 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