You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucenenet.apache.org by GitBox <gi...@apache.org> on 2022/11/07 05:40:51 UTC

[GitHub] [lucenenet] NightOwl888 commented on a diff in pull request #746: fix: Aligned disposable patterns

NightOwl888 commented on code in PR #746:
URL: https://github.com/apache/lucenenet/pull/746#discussion_r1014982216


##########
src/Lucene.Net/Index/PrefixCodedTerms.cs:
##########
@@ -87,7 +87,7 @@ internal PrefixCodedTermsIterator(RAMFile buffer)
             public void Dispose()
             {
                 Dispose(true);
-                GC.SuppressFinalize(true);
+                GC.SuppressFinalize(this);

Review Comment:
   Good catch



##########
src/Lucene.Net.Spatial/Prefix/AbstractVisitingPrefixTreeFilter.cs:
##########
@@ -367,7 +367,7 @@ protected internal virtual void Scan(int scanDetailLevel)
         /// <summary>
         /// Used for <see cref="VNode.children"/>.
         /// </summary>
-        private class VNodeCellEnumerator : IEnumerator<VNode>
+        private sealed class VNodeCellEnumerator : IEnumerator<VNode>

Review Comment:
   Please add a comment to the end of this line ` // LUCENENET: Marked sealed` to indicate we intentionally diverged from Lucene here.



##########
src/Lucene.Net.Facet/Taxonomy/WriterCache/CollisionMap.cs:
##########
@@ -213,7 +213,7 @@ internal virtual int GetMemoryUsage()
             return memoryUsage;
         }
 
-        private class EntryEnumerator : IEnumerator<Entry>
+        private sealed class EntryEnumerator : IEnumerator<Entry>

Review Comment:
   Please add a comment to the end of this line ` // LUCENENET: Marked sealed` to indicate we intentionally diverged from Lucene here.



##########
src/Lucene.Net/Store/FSDirectory.cs:
##########
@@ -562,6 +562,7 @@ protected override void Dispose(bool disposing)
                         }
                     }
                 }
+                //base.Dispose(disposing); // Causes object to be disposed in some tests

Review Comment:
   Could you elaborate a bit more what is happening here? Also, please prefix the comment with `// LUCENENET:` so we know that it wasn't a Lucene comment.



##########
src/Lucene.Net/Support/Index/TaskMergeScheduler.cs:
##########
@@ -621,14 +621,23 @@ private void Run(CancellationToken cancellationToken)
 
             public void Dispose()
             {
-                if (_isDisposed)
+                Dispose(true);

Review Comment:
   For future reference, please do not waste any time on `TaskMergeScheduler`. It has been deprecated and will be removed before the release.



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

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