NightOwl888 commented on code in PR #837:
URL: https://github.com/apache/lucenenet/pull/837#discussion_r1167101103
##########
src/Lucene.Net/Search/IndexSearcher.cs:
##########
@@ -135,13 +134,31 @@ public IndexSearcher(IndexReader r, TaskScheduler
executor)
/// <seealso cref="IndexReaderContext"/>
/// <seealso cref="IndexReader.Context"/>
public IndexSearcher(IndexReaderContext context, TaskScheduler
executor)
+ : this(context, executor, allocateLeafSlices: executor is not null)
+ {
+ }
+
+ /// <summary>
+ /// LUCENENET specific constructor that can be used by the subclasses
to
+ /// control whether or not the leaf slices are allocated.
+ /// If you choose to skip allocating the leaf slices here, you must
+ /// initialize <see cref="m_leafSlices" /> in your subclass's
constructor, either
+ /// by calling <see cref="GetSlices"/> or using your own logic
Review Comment:
Let's be a bit more specific:
```
/// <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 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>
```
##########
src/Lucene.Net/Search/IndexSearcher.cs:
##########
@@ -560,6 +570,9 @@ protected virtual TopFieldDocs Search(Weight weight,
FieldDoc after, int nDocs,
ReentrantLock @lock = new ReentrantLock();
ExecutionHelper<TopFieldDocs> runner = new
ExecutionHelper<TopFieldDocs>(executor);
+ if (m_leafSlices == null)
Review Comment:
See my comment on line 474.
##########
src/Lucene.Net/Search/IndexSearcher.cs:
##########
@@ -464,6 +471,9 @@ protected virtual TopDocs Search(Weight weight, ScoreDoc
after, int nDocs)
ReentrantLock @lock = new ReentrantLock();
ExecutionHelper<TopDocs> runner = new
ExecutionHelper<TopDocs>(executor);
+ if (m_leafSlices == null)
+ throw new InvalidOperationException("m_leafSlices must not
be null and initialized in the constructor");
Review Comment:
Let's change this a bit:
```c#
throw new InvalidOperationException($"When the constructor is passed a
non-null {nameof(TaskScheduler)}, {nameof(m_leafSlices)} must also be set to a
non-null value in the constructor.");
```
##########
src/Lucene.Net/Search/IndexSearcher.cs:
##########
@@ -464,6 +471,9 @@ protected virtual TopDocs Search(Weight weight, ScoreDoc
after, int nDocs)
ReentrantLock @lock = new ReentrantLock();
ExecutionHelper<TopDocs> runner = new
ExecutionHelper<TopDocs>(executor);
+ if (m_leafSlices == null)
Review Comment:
Although it doesn't make any difference for arrays, all `null` equality
checks have all been converted to use `is` in #617.
The `is` keyword doesn't take any overrides of the `==` operator into
account, so it is not possible to circumvent our `null` check logic by passing
a class that has one.
##########
src/Lucene.Net/Search/IndexSearcher.cs:
##########
@@ -560,6 +570,9 @@ protected virtual TopFieldDocs Search(Weight weight,
FieldDoc after, int nDocs,
ReentrantLock @lock = new ReentrantLock();
ExecutionHelper<TopFieldDocs> runner = new
ExecutionHelper<TopFieldDocs>(executor);
+ if (m_leafSlices == null)
+ throw new InvalidOperationException("m_leafSlices must not
be null and initialized in the constructor");
Review Comment:
Let's change this a bit:
```c#
throw new InvalidOperationException($"When the constructor is passed a
non-null {nameof(TaskScheduler)}, {nameof(m_leafSlices)} must also be set to a
non-null value in the constructor.");
```
--
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]
