NightOwl888 commented on code in PR #837:
URL: https://github.com/apache/lucenenet/pull/837#discussion_r1167174378
##########
src/Lucene.Net/Search/IndexSearcher.cs:
##########
@@ -134,14 +134,45 @@ public IndexSearcher(IndexReader r, TaskScheduler
executor)
/// </summary>
/// <seealso cref="IndexReaderContext"/>
/// <seealso cref="IndexReader.Context"/>
- public IndexSearcher(IndexReaderContext context, TaskScheduler
executor)
+ public IndexSearcher(IndexReaderContext context, TaskScheduler?
executor)
+ : this(context, executor, allocateLeafSlices: executor is not null)
{
- if (Debugging.AssertsEnabled)
Debugging.Assert(context.IsTopLevel,"IndexSearcher's ReaderContext must be
topLevel for reader {0}", context.Reader);
+ }
+
+ /// <summary>
+ /// LUCENENET specific constructor that can be used by the subclasses
to
+ /// control whether the leaf slices are allocated in the base class or
subclass.
+ /// </summary>
+ /// <remarks>
+ /// If <paramref name="executor"/> is non-<c>null</c> and you choose
to skip allocating the leaf slices
+ /// (i.e. <paramref name="allocateLeafSlices"/> == <c>false</c>), you
must
+ /// set the <see cref="m_leafSlices"/> field in your subclass
constructor.
+ /// This is commonly done by calling <see
cref="GetSlices(IList{AtomicReaderContext})"/>
+ /// and using the result to set <see cref="m_leafSlices"/>. You may
wish to do this if you
+ /// have state to pass into your constructor and need to set it prior
to the call to
+ /// <see cref="GetSlices(IList{AtomicReaderContext})"/> so it is
available for use
+ /// as a member field or property inside a custom override of
+ /// <see cref="GetSlices(IList{AtomicReaderContext})"/>.
+ /// </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")]
+ protected IndexSearcher(IndexReaderContext context, TaskScheduler?
executor, bool allocateLeafSlices)
+ {
+ if (context is null)
+ throw new ArgumentNullException(nameof(context));
+
+ if (Debugging.AssertsEnabled)
Review Comment:
I usually try to keep these on the same line so they show up in a search.
Unless there is more than one assert in a block.
This is because in Lucene there is simply a call to `assert <the assert> :
<the assert message>`. Unfortunately, .NET implemented assertions as a method
and also doesn't have a way to turn asserts on and off, so we went this route
to minimize the performance impact. The if statement is just an optimization so
we don't call the `Assert` method during normal operation. This all equates to
a single statement in Java, so we keep it on the same line. Unless we have
multiple ifs in a row, which we optimize down to a single if.
##########
src/Lucene.Net/Search/IndexSearcher.cs:
##########
@@ -397,13 +432,9 @@ public virtual TopFieldDocs Search(Query query, int n,
Sort sort)
/// <see cref="BooleanQuery.MaxClauseCount"/> clauses.
</exception>
public virtual TopDocs SearchAfter(ScoreDoc after, Query query, int n,
Sort sort)
Review Comment:
after can be marked nullable
Also, let's update the docs to indicate `<exception
cref="ArgumentNullException"><paramref name="query"> is
<c>null</c></exception>`.
Not sure whether `sort` is nullable, that will take some digging.
##########
src/Lucene.Net/Search/IndexSearcher.cs:
##########
@@ -117,7 +117,7 @@ public IndexSearcher(IndexReader r)
/// <para/>
/// @lucene.experimental
/// </summary>
- public IndexSearcher(IndexReader r, TaskScheduler executor)
+ public IndexSearcher(IndexReader r, TaskScheduler? executor)
: this(r.Context, executor)
Review Comment:
Possible null reference exception here. Let's use `r?.Context`.
##########
src/Lucene.Net/Search/IndexSearcher.cs:
##########
@@ -424,13 +455,9 @@ public virtual TopDocs SearchAfter(ScoreDoc after, Query
query, int n, Sort sort
/// <see cref="BooleanQuery.MaxClauseCount"/> clauses.
</exception>
public virtual TopDocs SearchAfter(ScoreDoc after, Query query, Filter
filter, int n, Sort sort, bool doDocScores, bool doMaxScore)
Review Comment:
after can be marked nullable
##########
src/Lucene.Net/Search/IndexSearcher.cs:
##########
@@ -587,7 +620,7 @@ protected virtual TopFieldDocs Search(Weight weight,
FieldDoc after, int nDocs,
/// whether or not the fields in the returned <see cref="FieldDoc"/>
instances should
/// be set by specifying <paramref name="fillFields"/>.
/// </summary>
- protected virtual TopFieldDocs Search(IList<AtomicReaderContext>
leaves, Weight weight, FieldDoc after, int nDocs, Sort sort, bool fillFields,
bool doDocScores, bool doMaxScore)
+ protected virtual TopFieldDocs Search(IList<AtomicReaderContext>
leaves, Weight weight, FieldDoc? after, int nDocs, Sort sort, bool fillFields,
bool doDocScores, bool doMaxScore)
{
// single thread
Review Comment:
We need a null guard clause for weight here, since we access properties of
it.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
