NightOwl888 commented on code in PR #1049:
URL: https://github.com/apache/lucenenet/pull/1049#discussion_r1860892946
##########
src/Lucene.Net/Support/Analysis/TokenAttributes/Extensions/CharTermAttributeExtensions.cs:
##########
@@ -0,0 +1,73 @@
+using System;
+
+namespace Lucene.Net.Analysis.TokenAttributes.Extensions
+{
+ /*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+ /// <summary>
+ /// Extension methods on <see cref="ICharTermAttribute"/>.
+ /// </summary>
+ public static class CharTermAttributeExtensions
+ {
+ /// <summary>
+ /// Set number of valid characters (length of the term) in
+ /// the termBuffer array. Use this to truncate the termBuffer
+ /// or to synchronize with external manipulation of the termBuffer.
+ /// Note: to grow the size of the array,
+ /// use <see cref="ICharTermAttribute.ResizeBuffer(int)"/> first.
+ /// <para />
+ /// NOTE: This is exactly the same operation as calling the <see
cref="ICharTermAttribute.Length"/> setter, the primary
+ /// difference is that this method returns a reference to the current
object so it can be chained.
+ /// <code>
+ /// obj.SetLength(30).Append("hey you");
+ /// </code>
+ /// </summary>
+ /// <param name="length">The truncated length</param>
+ public static ICharTermAttribute SetLength(this ICharTermAttribute
termAttr, int length)
+ {
+ if (termAttr is null)
+ {
+ throw new ArgumentNullException(nameof(termAttr));
+ }
+
+ termAttr.Length = length;
+ return termAttr;
+ }
+
+ /// <summary>
+ /// Sets the length of the termBuffer to zero.
+ /// Use this method before appending contents.
+ /// <para />
+ /// NOTE: This is exactly the same operation as calling <see
cref="Lucene.Net.Util.IAttribute.Clear()"/>, the primary
+ /// difference is that this method returns a reference to the current
object so it can be chained.
+ /// <code>
+ /// obj.SetEmpty().Append("hey you");
+ /// </code>
+ /// </summary>
+ public static ICharTermAttribute SetEmpty(this ICharTermAttribute
termAttr)
Review Comment:
Please change the method signature to return the concrete type, not the
interface type.
```c#
public static T SetEmpty<T>(this T termAttr) where T : ICharTermAttribute
```
##########
src/Lucene.Net/Analysis/TokenAttributes/CharTermAttributeImpl.cs:
##########
@@ -107,32 +107,27 @@ private void GrowTermBuffer(int newSize)
}
}
- int ICharTermAttribute.Length { get => Length; set =>
SetLength(value); }
+ int ICharTermAttribute.Length
+ {
+ get => Length;
+ set => Length = value;
+ }
int ICharSequence.Length => Length;
public int Length
{
get => termLength;
- set => SetLength(value);
- }
-
- public CharTermAttribute SetLength(int length)
- {
- // LUCENENET: added guard clause
- if (length < 0)
- throw new ArgumentOutOfRangeException(nameof(length), length,
$"{nameof(length)} must not be negative.");
- if (length > termBuffer.Length)
- throw new ArgumentOutOfRangeException(nameof(length), length,
"length " + length + " exceeds the size of the termBuffer (" +
termBuffer.Length + ")");
-
- termLength = length;
- return this;
- }
+ set
+ {
+ // LUCENENET: added guard clause
+ if (value < 0)
+ throw new ArgumentOutOfRangeException(nameof(value),
value, $"{nameof(value)} must not be negative.");
+ if (value > termBuffer.Length)
+ throw new ArgumentOutOfRangeException(nameof(value),
value, "length " + value + " exceeds the size of the termBuffer (" +
termBuffer.Length + ")");
Review Comment:
Please change this to use string interpolation. This appears to be the only
one that hasn't been converted in this class.
##########
src/Lucene.Net/Support/Analysis/TokenAttributes/Extensions/CharTermAttributeExtensions.cs:
##########
@@ -0,0 +1,73 @@
+using System;
+
+namespace Lucene.Net.Analysis.TokenAttributes.Extensions
+{
+ /*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+ /// <summary>
+ /// Extension methods on <see cref="ICharTermAttribute"/>.
+ /// </summary>
+ public static class CharTermAttributeExtensions
+ {
+ /// <summary>
+ /// Set number of valid characters (length of the term) in
+ /// the termBuffer array. Use this to truncate the termBuffer
+ /// or to synchronize with external manipulation of the termBuffer.
+ /// Note: to grow the size of the array,
+ /// use <see cref="ICharTermAttribute.ResizeBuffer(int)"/> first.
+ /// <para />
+ /// NOTE: This is exactly the same operation as calling the <see
cref="ICharTermAttribute.Length"/> setter, the primary
+ /// difference is that this method returns a reference to the current
object so it can be chained.
+ /// <code>
+ /// obj.SetLength(30).Append("hey you");
+ /// </code>
+ /// </summary>
+ /// <param name="length">The truncated length</param>
+ public static ICharTermAttribute SetLength(this ICharTermAttribute
termAttr, int length)
Review Comment:
Please change the method signature to return the concrete type, not the
interface type.
```c#
public static T SetLength<T>(this T termAttr, int length) where T :
ICharTermAttribute
```
##########
src/Lucene.Net.Analysis.Common/Analysis/NGram/NGramTokenFilter.cs:
##########
@@ -136,6 +140,10 @@ public int PositionLength
// LUCENENET specific - The interface requires this to be
implemented, since we added it to avoid casts.
public void CopyTo(IAttribute target) => _ = target;
Review Comment:
Now that `CopyTo()` and `Clear()` are not defined on `IAttribute`, they can
be removed to align with upstream.
##########
src/Lucene.Net.Analysis.Common/Analysis/NGram/NGramTokenFilter.cs:
##########
@@ -124,6 +124,10 @@ public int PositionIncrement
// LUCENENET specific - The interface requires this to be
implemented, since we added it to avoid casts.
public void CopyTo(IAttribute target) => _ = target;
Review Comment:
Now that `CopyTo()` and `Clear()` are not defined on `IAttribute`, they can
be removed to align with upstream.
##########
src/Lucene.Net/Analysis/TokenAttributes/CharTermAttribute.cs:
##########
@@ -285,5 +272,16 @@ public interface ICharTermAttribute : IAttribute,
ICharSequence, IAppendable
/// </summary>
/// <param name="value">The sequence of characters to append.</param>
ICharTermAttribute Append(ICharTermAttribute value);
+
+ /// <summary>
+ /// Clears the values in this attribute and resets it to its
+ /// default value.
+ /// </summary>
+ /// <remarks>
+ /// LUCENENET specific - This method is not part of the Java Lucene
API.
+ /// This was added to be a more consistent way to clear attributes
than SetEmpty().
+ /// </remarks>
+ /// <seealso
cref="CharTermAttributeExtensions.SetEmpty(ICharTermAttribute)"/>
+ void Clear();
Review Comment:
Since we are effectively replacing `SetEmpty()` with `Clear()` on this
attribute, please move the declaration above the `Append()` methods so it can
remain in the same order as upstream.
##########
src/Lucene.Net/Analysis/TokenAttributes/CharTermAttributeImpl.cs:
##########
@@ -107,32 +107,27 @@ private void GrowTermBuffer(int newSize)
}
}
- int ICharTermAttribute.Length { get => Length; set =>
SetLength(value); }
+ int ICharTermAttribute.Length
Review Comment:
These two explicit interface implementations for `Length` are superfluous
and can be removed. I suspect that cascading the call to the public `Length`
property would probably not perform as well as allowing the compiler to call
the property directly. This property is sealed by design, so there is no reason
we need to do this.
--
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]
