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


##########
src/Lucene.Net.Analysis.Common/Analysis/El/GreekStemmer.cs:
##########
@@ -994,39 +994,40 @@ private static int Rule22(char[] s, int len) // 
LUCENENET: CA1822: Mark members
             return len;
         }
 
-        /// <summary>
-        /// Checks if the word contained in the leading portion of char[] 
array , 
-        /// ends with the suffix given as parameter.
-        /// </summary>
-        /// <param name="s"> A char[] array that represents a word. </param>
-        /// <param name="len"> The length of the char[] array. </param>
-        /// <param name="suffix"> A <see cref="string"/> object to check if 
the word given ends with these characters. </param>
-        /// <returns> True if the word ends with the suffix given , false 
otherwise. </returns>
-        private static bool EndsWith(char[] s, int len, string suffix) // 
LUCENENET: CA1822: Mark members as static
-        {
-            int suffixLen = suffix.Length;
-            if (suffixLen > len)
-            {
-                return false;
-            }
-            for (int i = suffixLen - 1; i >= 0; i--)
-            {
-                if (s[len - (suffixLen - i)] != suffix[i])
-                {
-                    return false;
-                }
-            }
-
-            return true;
-        }
+        // LUCENENET: commented out unused private method
+        // /// <summary>
+        // /// Checks if the word contained in the leading portion of char[] 
array ,
+        // /// ends with the suffix given as parameter.
+        // /// </summary>
+        // /// <param name="s"> A char[] array that represents a word. </param>
+        // /// <param name="len"> The length of the char[] array. </param>
+        // /// <param name="suffix"> A <see cref="string"/> object to check if 
the word given ends with these characters. </param>
+        // /// <returns> True if the word ends with the suffix given , false 
otherwise. </returns>
+        // private static bool EndsWith(char[] s, int len, string suffix) // 
LUCENENET: CA1822: Mark members as static

Review Comment:
   If we use `ReadOnlySpan<char>`, then the `int len` parameter is redundant, 
since the span should already be sliced to the length when passed in. The whole 
point is to eliminate any extra `start` and `length` parameters, since the one 
method will do it all just by slicing. But IMO, it makes more sense to keep 
passing the `char[]` here (for now) along with `int` and `string` to match 
Lucene.
   
   > Note that we have a hybrid implementation of Snowball. We copied most of 
the bits from Lucene, but Snowball has a C# implementation that would be better 
to use. https://github.com/snowballstem/snowball/tree/master/csharp. The 
stemmer code is generated somehow, but I haven't looked at it in depth. We 
ended up using some of it because the Java code was using Reflection. But my 
point is there is a better optimized implementation that we can use in the 
future.



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

Reply via email to