You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucenenet.apache.org by "laimis (via GitHub)" <gi...@apache.org> on 2023/04/09 21:49:46 UTC

[GitHub] [lucenenet] laimis opened a new pull request, #815: BREAKING: Fix/virtual calls from constructors for AbstractBlockPackedWriter

laimis opened a new pull request, #815:
URL: https://github.com/apache/lucenenet/pull/815

   Continuation of fixes with virtual calls being made from constructors. The issue originally reported by SonarCloud scans: https://sonarcloud.io/project/issues?resolved=false&rules=csharpsquid%3AS1699&id=apache_lucenenet and referenced in this issue: https://github.com/apache/lucenenet/issues/670
   
   This one focuses on AbstractBlockPackedWriter, creating a private method that the constructor and public virtual methods can call.


-- 
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


[GitHub] [lucenenet] NightOwl888 commented on a diff in pull request #815: BREAKING: Fix/virtual calls from constructors for AbstractBlockPackedWriter

Posted by "NightOwl888 (via GitHub)" <gi...@apache.org>.
NightOwl888 commented on code in PR #815:
URL: https://github.com/apache/lucenenet/pull/815#discussion_r1162017329


##########
src/Lucene.Net/Util/Packed/AbstractBlockPackedWriter.cs:
##########
@@ -67,14 +67,22 @@ internal static void WriteVInt64(DataOutput @out, long i)
         protected AbstractBlockPackedWriter(DataOutput @out, int blockSize) // LUCENENET specific - marked protected instead of public
         {
             PackedInt32s.CheckBlockSize(blockSize, MIN_BLOCK_SIZE, MAX_BLOCK_SIZE);
-            Reset(@out);
+            ResetInternal(@out); // LUCENENET specific - calling private method instead of virtual Reset
             m_values = new long[blockSize];
         }
 
         /// <summary>
-        /// Reset this writer to wrap <paramref name="out"/>. The block size remains unchanged. </summary>
+        /// Reset this writer to wrap <paramref name="out"/>. The block size remains unchanged.
+        ///
+        /// NOTE: When overriding this method, be aware that the constructor of this class calls 
+        /// a private method and not this virtual method. So if you need to override
+        /// the behavior during the initialization, call your own private method from the constructor
+        /// with whatever custom behavior you need.
+        /// </summary>
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        public virtual void Reset(DataOutput @out)
+        public virtual void Reset(DataOutput @out) => ResetInternal(@out);
+
+        private void ResetInternal(DataOutput @out)

Review Comment:
   Let's add `[MethodImpl(MethodImplOptions.AggressiveInlining)]` here.



-- 
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


[GitHub] [lucenenet] laimis merged pull request #815: BREAKING: Fix/virtual calls from constructors for AbstractBlockPackedWriter

Posted by "laimis (via GitHub)" <gi...@apache.org>.
laimis merged PR #815:
URL: https://github.com/apache/lucenenet/pull/815


-- 
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