NightOwl888 commented on code in PR #690:
URL: https://github.com/apache/lucenenet/pull/690#discussion_r996470519
##########
src/Lucene.Net.Analysis.Common/Analysis/Util/AbstractAnalysisFactory.cs:
##########
@@ -177,15 +177,15 @@ public virtual string Get(IDictionary<string, string>
args, string name, ICollec
/// <summary>
/// NOTE: This was requireInt() in Lucene
/// </summary>
- protected int RequireInt32(IDictionary<string, string> args, string
name)
+ protected virtual int RequireInt32(IDictionary<string, string> args,
string name)
Review Comment:
This is marked final in Lucene 4.8.1, so it should not be virtual. This
should also remain an instance method, since it is meant for subclasses to use.
```java
protected final int requireInt(Map<String,String> args, String name) {
return Integer.parseInt(require(args, name));
}
```
##########
src/Lucene.Net.Analysis.Common/Analysis/Util/AbstractAnalysisFactory.cs:
##########
@@ -329,7 +329,7 @@ protected CultureInfo GetCulture(IDictionary<string,
string> args, string name,
/// Returns as <see cref="CharArraySet"/> from wordFiles, which
/// can be a comma-separated list of filenames
/// </summary>
- protected CharArraySet GetWordSet(IResourceLoader loader, string
wordFiles, bool ignoreCase)
+ protected virtual CharArraySet GetWordSet(IResourceLoader loader,
string wordFiles, bool ignoreCase)
Review Comment:
This is marked final in Lucene 4.8.1, so it should not be virtual. This
should also remain an instance method, since it is meant for subclasses to use.
```java
protected final CharArraySet getWordSet(ResourceLoader loader,
String wordFiles, boolean ignoreCase) throws IOException {
```
##########
src/Lucene.Net.Tests.Analysis.Phonetic/Language/MatchRatingApproachEncoderTest.cs:
##########
@@ -90,19 +90,19 @@ public void
TestAccentRemoval_NullValue_ReturnNullSuccessfully()
[Test]
public void
TestRemoveSingleDoubleConsonants_BUBLE_RemovedSuccessfully()
{
- Assert.AreEqual("BUBLE",
this.StringEncoder.RemoveDoubleConsonants("BUBBLE"));
+ Assert.AreEqual("BUBLE",
MatchRatingApproachEncoder.RemoveDoubleConsonants("BUBBLE"));
Review Comment:
Encoder instances are meant to be interchangeable, so we need to keep these
methods available through `this.StringEncoder`, not as a static methods. I am
referring to all of the methods under test in this class.
https://github.com/apache/commons-codec/blob/1.9/src/test/java/org/apache/commons/codec/language/MatchRatingApproachEncoderTest.java#L87
##########
src/Lucene.Net/Index/TermVectorsConsumerPerField.cs:
##########
@@ -141,7 +141,7 @@ internal override bool Start(IIndexableField[] fields, int
count)
}
[MethodImpl(MethodImplOptions.NoInlining)]
- public void Abort()
+ public static void Abort() // LUCENENET: CA1822: Mark members as static
Review Comment:
This method was not marked final in Lucene 4.8.0, therefore it should be
made `virtual`.
```java
public void abort() {}
```
##########
src/Lucene.Net.Analysis.Common/Analysis/Util/AbstractAnalysisFactory.cs:
##########
@@ -195,12 +195,12 @@ protected int GetInt32(IDictionary<string, string> args,
string name, int defaul
return defaultVal;
}
- protected bool RequireBoolean(IDictionary<string, string> args, string
name)
+ protected virtual bool RequireBoolean(IDictionary<string, string>
args, string name)
{
return bool.Parse(Require(args, name));
}
- protected bool GetBoolean(IDictionary<string, string> args, string
name, bool defaultVal)
+ protected virtual bool GetBoolean(IDictionary<string, string> args,
string name, bool defaultVal)
Review Comment:
This is marked final in Lucene 4.8.1, so it should not be virtual. This
should also remain an instance method, since it is meant for subclasses to use.
```java
protected final boolean getBoolean(Map<String,String> args, String name,
boolean defaultVal) {
String s = args.remove(name);
return s == null ? defaultVal : Boolean.parseBoolean(s);
}
```
##########
src/Lucene.Net.Analysis.Common/Analysis/Util/AbstractAnalysisFactory.cs:
##########
@@ -195,12 +195,12 @@ protected int GetInt32(IDictionary<string, string> args,
string name, int defaul
return defaultVal;
}
- protected bool RequireBoolean(IDictionary<string, string> args, string
name)
+ protected virtual bool RequireBoolean(IDictionary<string, string>
args, string name)
Review Comment:
This is marked final in Lucene 4.8.1, so it should not be virtual. This
should also remain an instance method, since it is meant for subclasses to use.
```java
protected final boolean requireBoolean(Map<String,String> args, String
name) {
return Boolean.parseBoolean(require(args, name));
}
```
##########
src/Lucene.Net.Analysis.Common/Analysis/Util/AbstractAnalysisFactory.cs:
##########
@@ -177,15 +177,15 @@ public virtual string Get(IDictionary<string, string>
args, string name, ICollec
/// <summary>
/// NOTE: This was requireInt() in Lucene
/// </summary>
- protected int RequireInt32(IDictionary<string, string> args, string
name)
+ protected virtual int RequireInt32(IDictionary<string, string> args,
string name)
{
return int.Parse(Require(args, name),
CultureInfo.InvariantCulture);
}
/// <summary>
/// NOTE: This was getInt() in Lucene
/// </summary>
- protected int GetInt32(IDictionary<string, string> args, string name,
int defaultVal)
+ protected virtual int GetInt32(IDictionary<string, string> args,
string name, int defaultVal)
Review Comment:
This is marked final in Lucene 4.8.1, so it should not be virtual. This
should also remain an instance method, since it is meant for subclasses to use.
```java
protected final int getInt(Map<String,String> args, String name, int
defaultVal) {
String s = args.remove(name);
return s == null ? defaultVal : Integer.parseInt(s);
}
```
##########
src/Lucene.Net.Analysis.Common/Analysis/Util/AbstractAnalysisFactory.cs:
##########
@@ -213,15 +213,15 @@ protected bool GetBoolean(IDictionary<string, string>
args, string name, bool de
/// <summary>
/// NOTE: This was requireFloat() in Lucene
/// </summary>
- protected float RequireSingle(IDictionary<string, string> args, string
name)
+ protected virtual float RequireSingle(IDictionary<string, string>
args, string name)
Review Comment:
This is marked final in Lucene 4.8.1, so it should not be virtual. This
should also remain an instance method, since it is meant for subclasses to use.
```java
protected final float requireFloat(Map<String,String> args, String name) {
return Float.parseFloat(require(args, name));
}
```
##########
src/Lucene.Net.Analysis.Common/Analysis/Util/AbstractAnalysisFactory.cs:
##########
@@ -213,15 +213,15 @@ protected bool GetBoolean(IDictionary<string, string>
args, string name, bool de
/// <summary>
/// NOTE: This was requireFloat() in Lucene
/// </summary>
- protected float RequireSingle(IDictionary<string, string> args, string
name)
+ protected virtual float RequireSingle(IDictionary<string, string>
args, string name)
{
return float.Parse(Require(args, name),
CultureInfo.InvariantCulture);
}
/// <summary>
/// NOTE: This was getFloat() in Lucene
/// </summary>
- protected float GetSingle(IDictionary<string, string> args, string
name, float defaultVal)
+ protected virtual float GetSingle(IDictionary<string, string> args,
string name, float defaultVal)
Review Comment:
This is marked final in Lucene 4.8.1, so it should not be virtual. This
should also remain an instance method, since it is meant for subclasses to use.
```java
protected final float getFloat(Map<String,String> args, String name, float
defaultVal) {
String s = args.remove(name);
return s == null ? defaultVal : Float.parseFloat(s);
}
```
##########
src/Lucene.Net.Analysis.Common/Analysis/Util/AbstractAnalysisFactory.cs:
##########
@@ -285,7 +285,7 @@ public virtual ISet<string> GetSet(IDictionary<string,
string> args, string name
/// <summary>
/// Compiles a pattern for the value of the specified argument key
<paramref name="name"/>
/// </summary>
- protected Regex GetPattern(IDictionary<string, string> args, string
name)
+ protected virtual Regex GetPattern(IDictionary<string, string> args,
string name)
Review Comment:
This is marked final in Lucene 4.8.1, so it should not be virtual. This
should also remain an instance method, since it is meant for subclasses to use.
```java
protected final Pattern getPattern(Map<String,String> args, String name) {
```
##########
src/Lucene.Net.Analysis.Common/Analysis/Util/AbstractAnalysisFactory.cs:
##########
@@ -351,7 +351,7 @@ protected CharArraySet GetWordSet(IResourceLoader loader,
string wordFiles, bool
/// <summary>
/// Returns the resource's lines (with content treated as UTF-8)
/// </summary>
- protected IList<string> GetLines(IResourceLoader loader, string
resource)
+ protected virtual IList<string> GetLines(IResourceLoader loader,
string resource)
Review Comment:
This is marked final in Lucene 4.8.1, so it should not be virtual. This
should also remain an instance method, since it is meant for subclasses to use.
```java
protected final List<String> getLines(ResourceLoader loader, String
resource) throws IOException {
return WordlistLoader.getLines(loader.openResource(resource),
StandardCharsets.UTF_8);
}
```
##########
src/Lucene.Net.Analysis.Common/Analysis/Util/AbstractAnalysisFactory.cs:
##########
@@ -388,7 +388,7 @@ protected CharArraySet GetSnowballWordSet(IResourceLoader
loader, string wordFil
/// </summary>
/// <param name="fileNames"> the string containing file names </param>
/// <returns> a list of file names with the escaping backslashed
removed </returns>
- protected IList<string> SplitFileNames(string fileNames)
+ protected virtual IList<string> SplitFileNames(string fileNames)
Review Comment:
This is marked final in Lucene 4.8.1, so it should not be virtual. This
should also remain an instance method, since it is meant for subclasses to use.
```java
protected final List<String> splitFileNames(String fileNames) {
```
##########
src/Lucene.Net.Analysis.Common/Analysis/Util/AbstractAnalysisFactory.cs:
##########
@@ -360,7 +360,7 @@ protected IList<string> GetLines(IResourceLoader loader,
string resource)
/// Same as <see cref="GetWordSet(IResourceLoader, string, bool)"/>,
/// except the input is in snowball format.
/// </summary>
- protected CharArraySet GetSnowballWordSet(IResourceLoader loader,
string wordFiles, bool ignoreCase)
+ protected virtual CharArraySet GetSnowballWordSet(IResourceLoader
loader, string wordFiles, bool ignoreCase)
Review Comment:
This is marked final in Lucene 4.8.1, so it should not be virtual. This
should also remain an instance method, since it is meant for subclasses to use.
```java
protected final CharArraySet getSnowballWordSet(ResourceLoader loader,
String wordFiles, boolean ignoreCase) throws IOException {
```
##########
src/Lucene.Net.Analysis.Common/Analysis/Util/CharacterUtils.cs:
##########
@@ -261,7 +261,7 @@ public int ToCodePoints(char[] src, int srcOff, int srcLen,
int[] dest, int dest
/// <summary>
/// Converts a sequence of unicode code points to a sequence of .NET
characters. </summary>
/// <returns> the number of chars written to the destination buffer
</returns>
- public int ToChars(int[] src, int srcOff, int srcLen, char[] dest, int
destOff)
+ public virtual int ToChars(int[] src, int srcOff, int srcLen, char[]
dest, int destOff)
Review Comment:
This is marked final in Lucene 4.8.1, so it should not be virtual. This
should also remain an instance method, since it is meant for subclasses to use.
```java
public final int toChars(int[] src, int srcOff, int srcLen, char[] dest,
int destOff) {
```
##########
src/Lucene.Net.TestFramework/Support/Util/LuceneRandomSeedInitializer.cs:
##########
@@ -44,7 +44,7 @@ internal class LuceneRandomSeedInitializer
/// <param name="seed">The random seed for a new <see cref="Random"/>
instance.
/// Note this is a subclass of <see cref="Random"/>, since the default
doesn't produce consistent results across platforms.</param>
/// <returns><c>true</c> if the seed was found in context;
<c>false</c> if the seed was generated.</returns>
- private bool TryGetRandomSeedsFromContext(Test test, out long seed,
out long? testSeed)
+ private static bool TryGetRandomSeedsFromContext(Test test, out long
seed, out long? testSeed) // LUCENENET: CA1822: Mark members as static
Review Comment:
No need for a comment in a Support folder. At least for stuff that we
created as a shim to make Lucene work. But do be aware that there are 2
packages in Benchmark and one in Phonetic that we have made part of the
assembly instead of creating separate NuGet packages and repositories to
maintain.
##########
src/Lucene.Net/Index/FreqProxTermsWriterPerField.cs:
##########
@@ -387,7 +387,7 @@ internal override int BytesPerPosting()
}
[MethodImpl(MethodImplOptions.NoInlining)]
- public void Abort()
+ public static void Abort()
Review Comment:
This method was not marked final in Lucene 4.8.0, therefore it should be
made `virtual`.
```java
public void abort() {}
```
##########
src/Lucene.Net/Index/DocumentsWriterFlushQueue.cs:
##########
@@ -299,7 +299,7 @@ protected FlushTicket(FrozenBufferedUpdates frozenUpdates)
indexWriter.PublishFlushedSegment(newSegment.segmentInfo,
segmentUpdates, globalPacket);
}
- protected void FinishFlush(IndexWriter indexWriter, FlushedSegment
newSegment, FrozenBufferedUpdates bufferedUpdates)
+ protected virtual void FinishFlush(IndexWriter indexWriter,
FlushedSegment newSegment, FrozenBufferedUpdates bufferedUpdates)
Review Comment:
This is marked final in Lucene 4.8.0, so should not be made virtual. Also,
it is more sensible as an instance method.
```java
protected final void finishFlush(IndexWriter indexWriter, FlushedSegment
newSegment, FrozenBufferedUpdates bufferedUpdates)
throws IOException {
```
##########
src/Lucene.Net.TestFramework/Support/Util/LuceneTestFrameworkInitializer.cs:
##########
@@ -276,7 +276,7 @@ internal void DoTestFrameworkTearDown()
/// // tight loop with many invocations.
/// </code>
/// </summary>
- protected Random Random => LuceneTestCase.Random;
+ protected static Random Random => LuceneTestCase.Random; // LUCENENET:
CA1822: Mark members as static
Review Comment:
This is just meant to be a shortcut so we don't have to access the static
`LuceneTestCase.Random` method in a subclass of
`LuceneTestFrameworkInitializer`. It should be an instance property, and should
not be `virtual`.
--
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
