You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucenenet.apache.org by la...@apache.org on 2023/04/09 04:00:59 UTC

[lucenenet] branch master updated: BREAKING: remove virtual call from the constructor for RollingBuffer and related classes (#809)

This is an automated email from the ASF dual-hosted git repository.

laimis pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/lucenenet.git


The following commit(s) were added to refs/heads/master by this push:
     new 189bd5e28 BREAKING: remove virtual call from the constructor for RollingBuffer and related classes (#809)
189bd5e28 is described below

commit 189bd5e28e25d67cc73f2d9ab967de92ed48fa73
Author: Laimonas Simutis <la...@gmail.com>
AuthorDate: Sat Apr 8 21:00:54 2023 -0700

    BREAKING: remove virtual call from the constructor for RollingBuffer and related classes (#809)
    
    * remove virtual call from the constructor and use item factory to create instances
---
 .../Analysis/LookaheadTokenFilter.cs               | 24 ++++------
 .../Analysis/MockGraphTokenFilter.cs               |  8 +---
 .../Analysis/MockRandomLookaheadTokenFilter.cs     |  9 ++--
 .../Analysis/TestLookaheadTokenFilter.cs           |  9 ++--
 .../Analysis/TrivialLookaheadFilter.cs             |  9 ++--
 .../Analysis/TestLookaheadTokenFilter.cs           | 11 ++---
 .../Analysis/TrivialLookaheadFilter.cs             |  9 ++--
 src/Lucene.Net.Tests/Util/TestRollingBuffer.cs     | 23 +++++-----
 src/Lucene.Net/Analysis/TokenStreamToAutomaton.cs  | 15 ++----
 src/Lucene.Net/Util/RollingBuffer.cs               | 53 ++++++++++++----------
 10 files changed, 71 insertions(+), 99 deletions(-)

diff --git a/src/Lucene.Net.TestFramework/Analysis/LookaheadTokenFilter.cs b/src/Lucene.Net.TestFramework/Analysis/LookaheadTokenFilter.cs
index c7675997c..e8b241f02 100644
--- a/src/Lucene.Net.TestFramework/Analysis/LookaheadTokenFilter.cs
+++ b/src/Lucene.Net.TestFramework/Analysis/LookaheadTokenFilter.cs
@@ -1,6 +1,7 @@
 using Lucene.Net.Analysis.TokenAttributes;
 using Lucene.Net.Diagnostics;
 using Lucene.Net.Util;
+using System;
 using System.Collections.Generic;
 using Console = Lucene.Net.Util.SystemConsole;
 using JCG = J2N.Collections.Generic;
@@ -50,7 +51,7 @@ namespace Lucene.Net.Analysis
         /// </summary>
         // LUCENENET NOTE: This class was originally marked protected, but was made public because of
         // inconsistent accessibility issues with using it as a generic constraint.
-        public class Position : RollingBuffer.IResettable
+        public class Position : IResettable
         {
             // Buffered input tokens at this position:
             public IList<AttributeSource.State> InputTokens { get; private set; } = new JCG.List<AttributeSource.State>();
@@ -120,11 +121,12 @@ namespace Lucene.Net.Analysis
 
         // LUCENENET specific - moved Position class to a non-generic class named LookaheadTokenFilter so we can refer to
         // it without referring to the generic closing type.
+        // removed virtual NewPosition() method and added factory in the constructor
 
-        protected internal LookaheadTokenFilter(TokenStream input)
+        protected internal LookaheadTokenFilter(TokenStream input, IRollingBufferItemFactory<T> itemFactory)
             : base(input)
         {
-            m_positions = new RollingBufferAnonymousClass(this);
+            m_positions = new RollingBufferAnonymousClass(itemFactory);
             m_posIncAtt = AddAttribute<IPositionIncrementAttribute>();
             m_posLenAtt = AddAttribute<IPositionLengthAttribute>();
             m_offsetAtt = AddAttribute<IOffsetAttribute>();
@@ -157,23 +159,15 @@ namespace Lucene.Net.Analysis
         {
         }
 
-        protected abstract T NewPosition();
-
         protected readonly RollingBuffer<T> m_positions;
 
         private sealed class RollingBufferAnonymousClass : RollingBuffer<T>
         {
-            private readonly LookaheadTokenFilter<T> outerInstance;
-
-            public RollingBufferAnonymousClass(LookaheadTokenFilter<T> outerInstance)
-                : base(outerInstance.NewPosition)
-            {
-                this.outerInstance = outerInstance;
-            }
-
-            protected override T NewInstance()
+            // LUCENENET specific - adjusted to accept factory as a parameter
+            // instead of using NewInstance virtual
+            public RollingBufferAnonymousClass(IRollingBufferItemFactory<T> itemFactory)
+                : base(itemFactory)
             {
-                return outerInstance.NewPosition();
             }
         }
 
diff --git a/src/Lucene.Net.TestFramework/Analysis/MockGraphTokenFilter.cs b/src/Lucene.Net.TestFramework/Analysis/MockGraphTokenFilter.cs
index a5b6dc789..e52443d75 100644
--- a/src/Lucene.Net.TestFramework/Analysis/MockGraphTokenFilter.cs
+++ b/src/Lucene.Net.TestFramework/Analysis/MockGraphTokenFilter.cs
@@ -42,18 +42,14 @@ namespace Lucene.Net.Analysis
         private readonly long seed;
         private Random random;
 
+        // LUCENENET specific - removed NewPosition override and using factory instead
         public MockGraphTokenFilter(Random random, TokenStream input)
-            : base(input)
+            : base(input, RollingBufferItemFactory<LookaheadTokenFilter.Position>.Default)
         {
             seed = random.NextInt64();
             termAtt = AddAttribute<ICharTermAttribute>();
         }
 
-        protected override Position NewPosition()
-        {
-            return new Position();
-        }
-
         protected override void AfterPosition()
         {
             if (DEBUG)
diff --git a/src/Lucene.Net.TestFramework/Analysis/MockRandomLookaheadTokenFilter.cs b/src/Lucene.Net.TestFramework/Analysis/MockRandomLookaheadTokenFilter.cs
index 1bdcd7be8..f979cc8af 100644
--- a/src/Lucene.Net.TestFramework/Analysis/MockRandomLookaheadTokenFilter.cs
+++ b/src/Lucene.Net.TestFramework/Analysis/MockRandomLookaheadTokenFilter.cs
@@ -1,4 +1,5 @@
 using Lucene.Net.Analysis.TokenAttributes;
+using Lucene.Net.Util;
 using RandomizedTesting.Generators;
 using System;
 using System.Threading;
@@ -32,19 +33,15 @@ namespace Lucene.Net.Analysis
         private readonly J2N.Randomizer random;
         private readonly long seed;
 
+        // LUCENENET specific - removed NewPosition override and using factory instead
         public MockRandomLookaheadTokenFilter(Random random, TokenStream @in)
-            : base(@in)
+            : base(@in, RollingBufferItemFactory<LookaheadTokenFilter.Position>.Default)
         {
             this.termAtt = AddAttribute<ICharTermAttribute>();
             this.seed = random.NextInt64();
             this.random = new J2N.Randomizer(seed);
         }
 
-        protected override Position NewPosition()
-        {
-            return new Position();
-        }
-
         protected override void AfterPosition()
         {
             if (!m_end && random.Next(4) == 2)
diff --git a/src/Lucene.Net.Tests.TestFramework/Analysis/TestLookaheadTokenFilter.cs b/src/Lucene.Net.Tests.TestFramework/Analysis/TestLookaheadTokenFilter.cs
index a8729dde3..e59cc081c 100644
--- a/src/Lucene.Net.Tests.TestFramework/Analysis/TestLookaheadTokenFilter.cs
+++ b/src/Lucene.Net.Tests.TestFramework/Analysis/TestLookaheadTokenFilter.cs
@@ -1,4 +1,5 @@
 // Lucene version compatibility level 8.2.0
+using Lucene.Net.Util;
 using RandomizedTesting.Generators;
 using System;
 using Test = NUnit.Framework.TestAttribute;
@@ -43,16 +44,12 @@ namespace Lucene.Net.Analysis
 
         private sealed class NeverPeeksLookaheadTokenFilter : LookaheadTokenFilter<LookaheadTokenFilter.Position>
         {
+            // LUCENENET specific - removed NewPosition override and using factory instead
             public NeverPeeksLookaheadTokenFilter(TokenStream input)
-                : base(input)
+                : base(input, RollingBufferItemFactory<LookaheadTokenFilter.Position>.Default)
             {
             }
 
-            protected override Position NewPosition()
-            {
-                return new Position();
-            }
-
             public override bool IncrementToken()
             {
                 return NextToken();
diff --git a/src/Lucene.Net.Tests.TestFramework/Analysis/TrivialLookaheadFilter.cs b/src/Lucene.Net.Tests.TestFramework/Analysis/TrivialLookaheadFilter.cs
index 36207766e..f3e8db4f6 100644
--- a/src/Lucene.Net.Tests.TestFramework/Analysis/TrivialLookaheadFilter.cs
+++ b/src/Lucene.Net.Tests.TestFramework/Analysis/TrivialLookaheadFilter.cs
@@ -1,6 +1,7 @@
 // Lucene version compatibility level 8.2.0
 
 using Lucene.Net.Analysis.TokenAttributes;
+using Lucene.Net.Util;
 using System;
 using JCG = J2N.Collections.Generic;
 
@@ -34,19 +35,15 @@ namespace Lucene.Net.Analysis
 
         private int insertUpto;
 
+        // LUCENENET specific - removed NewPosition override and using factory instead
         public TrivialLookaheadFilter(TokenStream input)
-            : base(input)
+            : base(input, RollingBufferItemFactory<TestPosition>.Default)
         {
             termAtt = AddAttribute<ICharTermAttribute>();
             posIncAtt = AddAttribute<IPositionIncrementAttribute>();
             offsetAtt = AddAttribute<IOffsetAttribute>();
         }
 
-        protected override TestPosition NewPosition()
-        {
-            return new TestPosition();
-        }
-
         public override bool IncrementToken()
         {
             // At the outset, getMaxPos is -1. So we'll peek. When we reach the end of the sentence and go to the
diff --git a/src/Lucene.Net.Tests/Analysis/TestLookaheadTokenFilter.cs b/src/Lucene.Net.Tests/Analysis/TestLookaheadTokenFilter.cs
index 7e0f89d68..231e0fba3 100644
--- a/src/Lucene.Net.Tests/Analysis/TestLookaheadTokenFilter.cs
+++ b/src/Lucene.Net.Tests/Analysis/TestLookaheadTokenFilter.cs
@@ -1,4 +1,5 @@
-using NUnit.Framework;
+using Lucene.Net.Util;
+using NUnit.Framework;
 using RandomizedTesting.Generators;
 using System;
 
@@ -40,16 +41,12 @@ namespace Lucene.Net.Analysis
 
         private class NeverPeeksLookaheadTokenFilter : LookaheadTokenFilter<LookaheadTokenFilter.Position>
         {
+            // LUCENENET specific - removed NewPosition override and using factory instead
             public NeverPeeksLookaheadTokenFilter(TokenStream input)
-                : base(input)
+                : base(input, RollingBufferItemFactory<LookaheadTokenFilter.Position>.Default)
             {
             }
 
-            protected override Position NewPosition()
-            {
-                return new Position();
-            }
-
             public sealed override bool IncrementToken()
             {
                 return NextToken();
diff --git a/src/Lucene.Net.Tests/Analysis/TrivialLookaheadFilter.cs b/src/Lucene.Net.Tests/Analysis/TrivialLookaheadFilter.cs
index 78c6ab2e8..d2c2a3118 100644
--- a/src/Lucene.Net.Tests/Analysis/TrivialLookaheadFilter.cs
+++ b/src/Lucene.Net.Tests/Analysis/TrivialLookaheadFilter.cs
@@ -1,4 +1,5 @@
 using Lucene.Net.Analysis.TokenAttributes;
+using Lucene.Net.Util;
 using System;
 using System.Collections.Generic;
 using JCG = J2N.Collections.Generic;
@@ -33,19 +34,15 @@ namespace Lucene.Net.Analysis
 
         private int insertUpto;
 
+        // LUCENENET specific - removed NewPosition override and using factory instead
         internal TrivialLookaheadFilter(TokenStream input)
-            : base(input)
+            : base(input, RollingBufferItemFactory<TestPosition>.Default)
         {
             termAtt = AddAttribute<ICharTermAttribute>();
             posIncAtt = AddAttribute<IPositionIncrementAttribute>();
             offsetAtt = AddAttribute<IOffsetAttribute>();
         }
 
-        protected override TestPosition NewPosition()
-        {
-            return new TestPosition();
-        }
-
         public override bool IncrementToken()
         {
             // At the outset, getMaxPos is -1. So we'll peek. When we reach the end of the sentence and go to the
diff --git a/src/Lucene.Net.Tests/Util/TestRollingBuffer.cs b/src/Lucene.Net.Tests/Util/TestRollingBuffer.cs
index 3a655ea5b..2a4be4b2a 100644
--- a/src/Lucene.Net.Tests/Util/TestRollingBuffer.cs
+++ b/src/Lucene.Net.Tests/Util/TestRollingBuffer.cs
@@ -25,7 +25,7 @@ namespace Lucene.Net.Util
     [TestFixture]
     public class TestRollingBuffer : LuceneTestCase
     {
-        private class Position : RollingBuffer.IResettable
+        private class Position : IResettable
         {
             public int Pos { get; set; }
 
@@ -90,23 +90,22 @@ namespace Lucene.Net.Util
 
         private sealed class RollingBufferAnonymousClass : RollingBuffer<Position>
         {
+            // LUCENENET specific - removed NewPosition override and using factory instead
             public RollingBufferAnonymousClass()
-                : base(NewInstanceFunc)
+                : base(PositionFactory.Default)
             {
             }
 
-            public static Position NewInstanceFunc()
+            private class PositionFactory : IRollingBufferItemFactory<Position>
             {
-                Position pos = new Position();
-                pos.Pos = -1;
-                return pos;
-            }
+                public static readonly PositionFactory Default = new PositionFactory();
 
-            protected override Position NewInstance()
-            {
-                Position pos = new Position();
-                pos.Pos = -1;
-                return pos;
+                public Position Create(object rollingBuffer)
+                {
+                    Position pos = new Position();
+                    pos.Pos = -1;
+                    return pos;
+                }
             }
         }
     }
diff --git a/src/Lucene.Net/Analysis/TokenStreamToAutomaton.cs b/src/Lucene.Net/Analysis/TokenStreamToAutomaton.cs
index 9664c42d9..1631d9b3b 100644
--- a/src/Lucene.Net/Analysis/TokenStreamToAutomaton.cs
+++ b/src/Lucene.Net/Analysis/TokenStreamToAutomaton.cs
@@ -70,7 +70,7 @@ namespace Lucene.Net.Analysis
             set => this.unicodeArcs = value;
         }
 
-        private class Position : RollingBuffer.IResettable
+        private class Position : IResettable
         {
             // Any tokens that ended at our position arrive to this state:
             internal State arriving;
@@ -87,18 +87,9 @@ namespace Lucene.Net.Analysis
 
         private class Positions : RollingBuffer<Position>
         {
+            // LUCENENET specific - removed NewInstance override and using PositionsFactory to create instances
             public Positions()
-                : base(NewPosition) { }
-
-            protected override Position NewInstance()
-            {
-                return NewPosition();
-            }
-
-            private static Position NewPosition()
-            {
-                return new Position();
-            }
+                : base(RollingBufferItemFactory<Position>.Default) { }
         }
 
         /// <summary>
diff --git a/src/Lucene.Net/Util/RollingBuffer.cs b/src/Lucene.Net/Util/RollingBuffer.cs
index 48139bd32..519646e03 100644
--- a/src/Lucene.Net/Util/RollingBuffer.cs
+++ b/src/Lucene.Net/Util/RollingBuffer.cs
@@ -1,6 +1,5 @@
 using Lucene.Net.Diagnostics;
 using Lucene.Net.Support;
-using System;
 using System.Runtime.CompilerServices;
 
 namespace Lucene.Net.Util
@@ -23,17 +22,32 @@ namespace Lucene.Net.Util
      */
 
     /// <summary>
-    /// LUCENENET specific class to allow referencing static members of
-    /// <see cref="RollingBuffer{T}"/> without referencing its generic closing type.
+    /// Implement to reset an instance
     /// </summary>
-    public static class RollingBuffer
+    public interface IResettable
     {
-        /// <summary>
-        /// Implement to reset an instance
-        /// </summary>
-        public interface IResettable
+        void Reset();
+    }
+
+    /// <summary>
+    /// LUCENENET specific interface to allow overriding rolling buffer item creation
+    /// without having to call virtual methods from the constructor
+    /// </summary>
+    public interface IRollingBufferItemFactory<out T>
+    {
+        T Create(object rollingBuffer);
+    }
+
+    /// <summary>
+    /// LUCENENET specific class that provides default implementation for
+    /// <see cref="IRollingBufferItemFactory{T}"/>.
+    /// </summary>
+    public class RollingBufferItemFactory<T> : IRollingBufferItemFactory<T> where T : new()
+    {
+        public static RollingBufferItemFactory<T> Default { get; } = new RollingBufferItemFactory<T>();
+        public virtual T Create(object rollingBuffer)
         {
-            void Reset();
+            return new T();
         }
     }
 
@@ -43,8 +57,9 @@ namespace Lucene.Net.Util
     /// <para/>
     /// @lucene.internal
     /// </summary>
+    // LUCENENET specific - removed NewInstance override and using NewPosition as factory
     public abstract class RollingBuffer<T>
-        where T : RollingBuffer.IResettable
+        where T : IResettable
     {
         private T[] buffer = new T[8];
 
@@ -57,25 +72,17 @@ namespace Lucene.Net.Util
         // How many valid Position are held in the
         // array:
         private int count;
+        private IRollingBufferItemFactory<T> itemFactory;
 
-        protected RollingBuffer()
-        {
-            for (var idx = 0; idx < buffer.Length; idx++)
-            {
-                buffer[idx] = NewInstance(); // TODO GIVE ROLLING BUFFER A DELEGATE FOR NEW INSTANCE
-            }
-        }
-
-        protected RollingBuffer(Func<T> factory)
+        protected RollingBuffer(IRollingBufferItemFactory<T> itemFactory)
         {
+            this.itemFactory = itemFactory; // LUCENENET specific - storing factory for usage in class
             for (int idx = 0; idx < buffer.Length; idx++)
             {
-                buffer[idx] = factory();
+                buffer[idx] = itemFactory.Create(this);
             }
         }
 
-        protected abstract T NewInstance();
-
         public virtual void Reset()
         {
             nextWrite--;
@@ -127,7 +134,7 @@ namespace Lucene.Net.Util
                     Arrays.Copy(buffer, 0, newBuffer, buffer.Length - nextWrite, nextWrite);
                     for (int i = buffer.Length; i < newBuffer.Length; i++)
                     {
-                        newBuffer[i] = NewInstance();
+                        newBuffer[i] = this.itemFactory.Create(this); // LUCENENET specific - using factory to create new instance
                     }
                     nextWrite = buffer.Length;
                     buffer = newBuffer;