NightOwl888 commented on code in PR #1139:
URL: https://github.com/apache/lucenenet/pull/1139#discussion_r1987632978
##########
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:
In the upstream code, this method is being called instead of
`StemmerUtil.EndsWith()`. Let's reenable this and have it cascade the call to
`MemoryExtensions`, which is better optimized.
```c#
private static bool EndsWith(char[] s, int len, string suffix) // LUCENENET:
CA1822: Mark members as static
=> MemoryExtensions.EndsWith(s.AsSpan(0, len), suffix.AsSpan());
```
We can probably also replace the `StemmerUtil` implementation with the same
cascade. Although, it is probably more sensible for end users to call
`MemoryExtensions` directly.
##########
src/Lucene.Net.Analysis.Phonetic/Language/Bm/Rule.cs:
##########
@@ -157,19 +157,19 @@ private static IDictionary<NameType,
IDictionary<RuleType, IDictionary<string, I
return rules;
}
-#pragma warning disable IDE0051 // Remove unused private members
- private static bool Contains(ICharSequence chars, char input)
-#pragma warning restore IDE0051 // Remove unused private members
- {
- for (int i = 0; i < chars.Length; i++)
- {
- if (chars[i] == input)
- {
- return true;
- }
- }
- return false;
- }
+ // LUCENENET: commented out unused private method
+ // private static bool Contains(ICharSequence chars, char input)
+ // {
+ // for (int i = 0; i < chars.Length; i++)
+ // {
+ // if (chars[i] == input)
+ // {
+ // return true;
+ // }
+ // }
+ // return false;
+ // }
+
private static bool Contains(string chars, char input)
Review Comment:
Let's remove the 2 overloads of this entirely and change the `chars`
parameter to `ReadOnlySpan<char>` to match Java (this will be the closest
equivalent to `ICharSequence` once `OpenStringBuilder` has been completed).
We should also change the implementation to use `MemoryExtensions`. .NET
Core has an optimized version of this, but it is entirely missing in net462.
But there is a patch in ICU4N that we can put in this project:
https://github.com/NightOwl888/ICU4N/blob/6c3cc907d06d1651554a5707170ecb7c338f7f1a/src/ICU4N/Support/MemoryExtensions.cs#L19-L39.
Making this public is not ideal because of .NET Standard support, which may
or may not have a colliding method of the same name. Although, we could
potentially make a `J2N.Memory.MemoryExtensions` class so the namespace
`J2N.Memory` would have to be imported in order to use the patch.
Changing the implementation doesn't have to be part of this PR. Instead, a
new issue can be opened or a `LUCENENET TODO` added and it will be done as part
of the migration to `ReadOnlySpan<char>`.
--
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
