NightOwl888 commented on code in PR #821:
URL: https://github.com/apache/lucenenet/pull/821#discussion_r1164542904
##########
src/Lucene.Net.Benchmark/ByTask/Tasks/ReadTask.cs:
##########
@@ -44,6 +45,8 @@ namespace Lucene.Net.Benchmarks.ByTask.Tasks
/// <para/>
/// Other side effects: none.
/// </remarks>
+ [SuppressMessage("CodeQuality", "IDE0079:Remove unnecessary suppression",
Justification = "This is a SonarCloud issue")]
+ [SuppressMessage("CodeQuality", "S1699:Constructors should only call
non-overridable methods", Justification = "Required for continuity with
Lucene's design")]
public abstract class ReadTask : PerfTask
Review Comment:
Let's add a second constructor here with the signature:
```c#
protected ReadTask(PerfRunData runData, IQueryMaker queryMaker)
: base(runData)
{
this.queryMaker = queryMaker;
}
```
The original constructor can call the new one instead of the base class.
##########
src/Lucene.Net.Benchmark/ByTask/Tasks/WriteLineDocTask.cs:
##########
@@ -92,7 +93,8 @@ public class WriteLineDocTask : PerfTask
private readonly object lineFileLock = new object(); // LUCENENET
specific - lock to ensure writes don't collide for this instance
-
+ [SuppressMessage("CodeQuality", "IDE0079:Remove unnecessary
suppression", Justification = "This is a SonarCloud issue")]
+ [SuppressMessage("CodeQuality", "S1699:Constructors should only call
non-overridable methods", Justification = "Required for continuity with
Lucene's design")]
public WriteLineDocTask(PerfRunData runData)
Review Comment:
This one can be fixed by changing the `lineFileOut` field:
```c#
protected readonly TextWriter m_lineFileOut;
```
And adding a constructor so the user can opt out of the call:
```c#
protected WriteLineDocTask(PerfRunData runData, bool performWriteHeader)
: base(runData)
{
Config config = runData.Config;
m_fname = config.Get("line.file.out", null);
if (m_fname is null)
{
throw new ArgumentException("line.file.out must be set");
}
Stream @out = StreamUtils.GetOutputStream(new FileInfo(m_fname));
m_lineFileOut = new StreamWriter(@out, Encoding.UTF8);
docMaker = runData.DocMaker;
// init fields
string f2r = config.Get("line.fields", null);
if (f2r is null)
{
fieldsToWrite = DEFAULT_FIELDS;
}
else
{
if (f2r.IndexOf(SEP) >= 0)
{
throw new ArgumentException("line.fields " + f2r + " should not
contain the separator char: " + SEP);
}
fieldsToWrite = f2r.Split(',').TrimEnd();
}
// init sufficient fields
sufficientFields = new bool[fieldsToWrite.Length];
string suff = config.Get("sufficient.fields", DEFAULT_SUFFICIENT_FIELDS);
if (",".Equals(suff, StringComparison.Ordinal))
{
checkSufficientFields = false;
}
else
{
checkSufficientFields = true;
ISet<string> sf = new JCG.HashSet<string>(suff.Split(',').TrimEnd());
for (int i = 0; i < fieldsToWrite.Length; i++)
{
if (sf.Contains(fieldsToWrite[i]))
{
sufficientFields[i] = true;
}
}
}
if (performWriteHeader)
{
WriteHeader(m_lineFileOut);
}
}
public WriteLineDocTask(PerfRunData runData)
: this(runData, performWriteHeader: true)
{
}
```
It can then be used like this:
```c#
public WriteEnwikiLineDocTask(PerfRunData runData)
: base(runData, performWriteHeader: false)
{
// Do my stuff here
WriteHeader(m_lineFileOut);
Stream @out = StreamUtils.GetOutputStream(CategoriesLineFile(new
FileInfo(m_fname)));
categoryLineFileOut = new StreamWriter(@out, Encoding.UTF8);
WriteHeader(categoryLineFileOut);
}
```
The trick is to move the existing code to the new protected constructor with
an opt out of the virtual call.
##########
src/Lucene.Net.Benchmark/ByTask/PerfRunData.cs:
##########
@@ -98,6 +99,8 @@ public class PerfRunData : IDisposable
private readonly IDictionary<string, object> perfObjects = new
Dictionary<string, object>();
// constructor
+ [SuppressMessage("CodeQuality", "IDE0079:Remove unnecessary
suppression", Justification = "This is a SonarCloud issue")]
+ [SuppressMessage("CodeQuality", "S1699:Constructors should only call
non-overridable methods", Justification = "Required for continuity with
Lucene's design")]
public PerfRunData(Config config)
Review Comment:
Let's add a protected constructor here with the signature
```c#
protected PerfRunData(Config config, bool performReinit)
```
This will give the user the ability to opt out of this call and call it
themselves after they set any state that they need.
--
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
