NightOwl888 commented on code in PR #1058:
URL: https://github.com/apache/lucenenet/pull/1058#discussion_r1997672927
##########
src/Lucene.Net.Spatial/Prefix/PrefixTreeStrategy.cs:
##########
@@ -159,14 +159,23 @@ public virtual Field[] CreateIndexableFields(IShape?
shape, double distErr)
}.Freeze();
/// <summary>
- /// Outputs the tokenString of a cell, and if its a leaf, outputs it
again with the leaf byte.
+ /// Outputs the tokenString of a cell, and if it's a leaf, outputs it
again with the leaf byte.
/// </summary>
/// <remarks>
/// LUCENENET specific - This class originally took an enumerator,
which meant that it could not
- /// be reused (since most IEnumerator implementations can only be
iterated once and throw on
- /// <see cref="System.Collections.IEnumerator.Reset()"/>). This class
has been modified to take an
- /// <c>IEnumerable<Cell></c> instead, which allows it to be
reused, per the TokenStream
- /// workflow contract of allowing <see cref="TokenStream.Reset()"/>
after <see cref="TokenStream.Close()"/>.
+ /// be reused after a call to <see cref="TokenStream.Close()"/> (which
was not present in the Lucene code).
+ /// This class has been modified to take an
<c>IEnumerable<Cell></c> instead, which allows it to be
+ /// reused, per the TokenStream workflow contract of allowing <see
cref="TokenStream.Reset()"/>
+ /// after <see cref="TokenStream.Close()"/>. To accomplish this, it
takes an enumerable and lazy-initializes
+ /// its enumerator upon the first call to <see
cref="TokenStream.Reset()"/>. Calling
+ /// <see cref="TokenStream.Close()"/> will dispose of the enumerator,
but it can be reinitialized by calling
+ /// <see cref="TokenStream.Reset()"/> again.
+ /// <para />
+ /// This implementation also expects that the enumerator will support
<see cref="IEnumerator{T}.Reset()"/>
Review Comment:
`Reset()` is on `IEnumerator`, not `IEnumerator<T>`. This XML doc doesn't
compile.
##########
src/Lucene.Net.Spatial/Prefix/PrefixTreeStrategy.cs:
##########
@@ -159,14 +159,23 @@ public virtual Field[] CreateIndexableFields(IShape?
shape, double distErr)
}.Freeze();
/// <summary>
- /// Outputs the tokenString of a cell, and if its a leaf, outputs it
again with the leaf byte.
+ /// Outputs the tokenString of a cell, and if it's a leaf, outputs it
again with the leaf byte.
/// </summary>
/// <remarks>
/// LUCENENET specific - This class originally took an enumerator,
which meant that it could not
- /// be reused (since most IEnumerator implementations can only be
iterated once and throw on
- /// <see cref="System.Collections.IEnumerator.Reset()"/>). This class
has been modified to take an
- /// <c>IEnumerable<Cell></c> instead, which allows it to be
reused, per the TokenStream
- /// workflow contract of allowing <see cref="TokenStream.Reset()"/>
after <see cref="TokenStream.Close()"/>.
+ /// be reused after a call to <see cref="TokenStream.Close()"/> (which
was not present in the Lucene code).
+ /// This class has been modified to take an
<c>IEnumerable<Cell></c> instead, which allows it to be
+ /// reused, per the TokenStream workflow contract of allowing <see
cref="TokenStream.Reset()"/>
+ /// after <see cref="TokenStream.Close()"/>. To accomplish this, it
takes an enumerable and lazy-initializes
+ /// its enumerator upon the first call to <see
cref="TokenStream.Reset()"/>. Calling
+ /// <see cref="TokenStream.Close()"/> will dispose of the enumerator,
but it can be reinitialized by calling
+ /// <see cref="TokenStream.Reset()"/> again.
+ /// <para />
+ /// This implementation also expects that the enumerator will support
<see cref="IEnumerator{T}.Reset()"/>
+ /// to avoid unnecessary allocations. Most BCL types in .NET support
this, but custom implementations may not.
+ /// For this reason, anyone that overrides
+ /// <see
cref="SpatialPrefixTree.GetCells(Spatial4n.Shapes.IShape?,int,bool,bool)"/>
must make sure to return
+ /// an implementation of <c>IList<Cell></c> that supports <see
cref="IEnumerator{T}.Reset()"/>.
Review Comment:
`Reset()` is on `IEnumerator`, not `IEnumerator<T>`. This XML doc doesn't
compile.
##########
src/Lucene.Net.Spatial/Prefix/Tree/SpatialPrefixTree.cs:
##########
@@ -178,6 +178,11 @@ protected internal virtual Cell GetCell(IPoint p, int
level)
/// <para/>
/// This implementation checks if shape is a <see cref="IPoint"/> and
if so returns
/// <see cref="GetCells(IPoint, int, bool)"/>.
+ /// <para />
+ /// LUCENENET NOTE: If overriding this method, make sure to return an
implementation of
+ /// <c>IList<Cell></c> whose <see
cref="IList{T}.GetEnumerator()"/> returns an enumerator
Review Comment:
`GetEnumerator()` is on `IEnumerable<T>`, not `IList<T>`. This XML doc
doesn't compile.
##########
src/Lucene.Net.Spatial/Prefix/Tree/SpatialPrefixTree.cs:
##########
@@ -178,6 +178,11 @@ protected internal virtual Cell GetCell(IPoint p, int
level)
/// <para/>
/// This implementation checks if shape is a <see cref="IPoint"/> and
if so returns
/// <see cref="GetCells(IPoint, int, bool)"/>.
+ /// <para />
+ /// LUCENENET NOTE: If overriding this method, make sure to return an
implementation of
+ /// <c>IList<Cell></c> whose <see
cref="IList{T}.GetEnumerator()"/> returns an enumerator
+ /// that supports being <see cref="IEnumerator{T}.Reset"/> without
throwing an exception.
Review Comment:
`Reset()` is on `IEnumerator`, not `IEnumerator<T>`. This XML doc doesn't
compile.
--
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
