You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucenenet.apache.org by "paulirwin (via GitHub)" <gi...@apache.org> on 2025/03/03 04:50:16 UTC
[PR] Use J2N methods instead of StringBuilder.Remove, #664 [lucenenet]
paulirwin opened a new pull request, #1132:
URL: https://github.com/apache/lucenenet/pull/1132
- [X] You've read the [Contributor Guide](https://github.com/apache/lucenenet/blob/main/CONTRIBUTING.md) and [Code of Conduct](https://www.apache.org/foundation/policies/conduct.html).
- [ ] You've included unit or integration tests for your change, where applicable.
- [ ] You've included inline docs for your change, where applicable.
- [X] There's an open issue for the PR that you are making. If you'd like to propose a change, please [open an issue](https://github.com/apache/lucenenet/issues/new/choose) to discuss the change or find an existing issue.
Use J2N methods instead of StringBuilder.Remove
Fixes #664
## Description
See #664 for details. In one case, the Remove/Insert was not able to be replaced with Replace and that was noted with a comment. Also, use of the pattern `sb.Remove(0, sb.Length)` was replaced with `sb.Clear()` instead of `sb.Delete(0, sb.Length)`, as it is more semantically understandable, less prone to mistakes, and has a few less checks internally than `Delete` cascading to `Remove` so it should be a little more efficient.
--
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-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Use J2N methods instead of StringBuilder.Remove, #664 [lucenenet]
Posted by "paulirwin (via GitHub)" <gi...@apache.org>.
paulirwin merged PR #1132:
URL: https://github.com/apache/lucenenet/pull/1132
--
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-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Use J2N methods instead of StringBuilder.Remove, #664 [lucenenet]
Posted by "NightOwl888 (via GitHub)" <gi...@apache.org>.
NightOwl888 commented on code in PR #1132:
URL: https://github.com/apache/lucenenet/pull/1132#discussion_r1979634666
##########
src/Lucene.Net.Analysis.Common/Analysis/De/GermanStemmer.cs:
##########
@@ -102,32 +103,32 @@ private void Strip(StringBuilder buffer)
{
if ((buffer.Length + substCount > 5) && buffer.ToString(buffer.Length - 2, buffer.Length - (buffer.Length - 2)).Equals("nd", StringComparison.Ordinal))
{
- buffer.Remove(buffer.Length - 2, buffer.Length - (buffer.Length - 2));
+ buffer.Delete(buffer.Length - 2, buffer.Length - (buffer.Length - 2));
}
else if ((buffer.Length + substCount > 4) && buffer.ToString(buffer.Length - 2, buffer.Length - (buffer.Length - 2)).Equals("em", StringComparison.Ordinal))
{
- buffer.Remove(buffer.Length - 2, buffer.Length - (buffer.Length - 2));
+ buffer.Delete(buffer.Length - 2, buffer.Length - (buffer.Length - 2));
Review Comment:
Since we don't need to calculate the end index, this can be simplified to:
```
buffer.Delete(buffer.Length - 2, 2); // LUCENENET: buffer.Length - (buffer.Length - 2) == 2
```
##########
src/Lucene.Net.Analysis.Common/Analysis/De/GermanStemmer.cs:
##########
@@ -102,32 +103,32 @@ private void Strip(StringBuilder buffer)
{
if ((buffer.Length + substCount > 5) && buffer.ToString(buffer.Length - 2, buffer.Length - (buffer.Length - 2)).Equals("nd", StringComparison.Ordinal))
{
- buffer.Remove(buffer.Length - 2, buffer.Length - (buffer.Length - 2));
+ buffer.Delete(buffer.Length - 2, buffer.Length - (buffer.Length - 2));
Review Comment:
Since we don't need to calculate the end index, this can be simplified to:
```
buffer.Delete(buffer.Length - 2, 2); // LUCENENET: buffer.Length - (buffer.Length - 2) == 2
```
--
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-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Use J2N methods instead of StringBuilder.Remove, #664 [lucenenet]
Posted by "paulirwin (via GitHub)" <gi...@apache.org>.
paulirwin commented on PR #1132:
URL: https://github.com/apache/lucenenet/pull/1132#issuecomment-6256242227
@NightOwl888 This is finally ready for review.
Previously I had posted that this was waiting on the OpenStringBuilder/TextBuilder work, but that turned out to be (somewhat) incorrect. It was really waiting on https://github.com/NightOwl888/J2N/issues/171, as you pointed out to me separately. Some of these stemmers could be moved to TextBuilder, but that would be a separate concern that should be justified independently via benchmarking.
This PR now ensures:
- Every plan `Remove` is now `Delete`, which clamps instead of throwing.
- `Remove().Insert()` pairs are now J2N's `Replace`, as #664 specified.
- The `Delete` clamping is in J2N alpha 53, which master already uses, so nothing is still blocked.
- `Remove(0, Length)` calls are now `Clear()` which has better semantics and avoids possible argument math issues.
--
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-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Use J2N methods instead of StringBuilder.Remove, #664 [lucenenet]
Posted by "paulirwin (via GitHub)" <gi...@apache.org>.
paulirwin commented on PR #1132:
URL: https://github.com/apache/lucenenet/pull/1132#issuecomment-2698235036
Converted to draft until OpenStringBuilder is in place
--
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-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Use J2N methods instead of StringBuilder.Remove, #664 [lucenenet]
Posted by "NightOwl888 (via GitHub)" <gi...@apache.org>.
NightOwl888 commented on code in PR #1132:
URL: https://github.com/apache/lucenenet/pull/1132#discussion_r4370753631
##########
src/Lucene.Net.Analysis.Common/Analysis/De/GermanStemmer.cs:
##########
@@ -102,32 +102,32 @@ private void Strip(StringBuilder buffer)
{
if ((buffer.Length + substCount > 5) && buffer.ToString(buffer.Length - 2, buffer.Length - (buffer.Length - 2)).Equals("nd", StringComparison.Ordinal))
{
- buffer.Remove(buffer.Length - 2, buffer.Length - (buffer.Length - 2));
+ buffer.Delete(buffer.Length - 2, buffer.Length - (buffer.Length - 2));
}
else if ((buffer.Length + substCount > 4) && buffer.ToString(buffer.Length - 2, buffer.Length - (buffer.Length - 2)).Equals("em", StringComparison.Ordinal))
{
- buffer.Remove(buffer.Length - 2, buffer.Length - (buffer.Length - 2));
+ buffer.Delete(buffer.Length - 2, buffer.Length - (buffer.Length - 2));
}
else if ((buffer.Length + substCount > 4) && buffer.ToString(buffer.Length - 2, buffer.Length - (buffer.Length - 2)).Equals("er", StringComparison.Ordinal))
{
- buffer.Remove(buffer.Length - 2, buffer.Length - (buffer.Length - 2));
+ buffer.Delete(buffer.Length - 2, buffer.Length - (buffer.Length - 2));
Review Comment:
Since we don't need to calculate the end index, this can be simplified to:
```
buffer.Remove(buffer.Length - 2, 2); // LUCENENET: buffer.Length - (buffer.Length - 2) == 2
```
##########
src/Lucene.Net.Analysis.Common/Analysis/De/GermanStemmer.cs:
##########
@@ -102,32 +103,32 @@ private void Strip(StringBuilder buffer)
{
if ((buffer.Length + substCount > 5) && buffer.ToString(buffer.Length - 2, buffer.Length - (buffer.Length - 2)).Equals("nd", StringComparison.Ordinal))
{
- buffer.Remove(buffer.Length - 2, buffer.Length - (buffer.Length - 2));
+ buffer.Delete(buffer.Length - 2, buffer.Length - (buffer.Length - 2));
Review Comment:
Since we don't need to calculate the end index, this can be simplified to:
```
buffer.Remove(buffer.Length - 2, 2); // LUCENENET: buffer.Length - (buffer.Length - 2) == 2
```
##########
src/Lucene.Net.Analysis.Common/Analysis/De/GermanStemmer.cs:
##########
@@ -102,32 +103,32 @@ private void Strip(StringBuilder buffer)
{
if ((buffer.Length + substCount > 5) && buffer.ToString(buffer.Length - 2, buffer.Length - (buffer.Length - 2)).Equals("nd", StringComparison.Ordinal))
{
- buffer.Remove(buffer.Length - 2, buffer.Length - (buffer.Length - 2));
+ buffer.Delete(buffer.Length - 2, buffer.Length - (buffer.Length - 2));
}
else if ((buffer.Length + substCount > 4) && buffer.ToString(buffer.Length - 2, buffer.Length - (buffer.Length - 2)).Equals("em", StringComparison.Ordinal))
{
- buffer.Remove(buffer.Length - 2, buffer.Length - (buffer.Length - 2));
+ buffer.Delete(buffer.Length - 2, buffer.Length - (buffer.Length - 2));
Review Comment:
Since we don't need to calculate the end index, this can be simplified to:
```
buffer.Remove(buffer.Length - 2, 2); // LUCENENET: buffer.Length - (buffer.Length - 2) == 2
```
##########
src/Lucene.Net.Analysis.Common/Analysis/Fr/FrenchStemmer.cs:
##########
@@ -23,14 +25,14 @@ namespace Lucene.Net.Analysis.Fr
*/
/// <summary>
- /// A stemmer for French words.
+ /// A stemmer for French words.
/// <para/>
/// The algorithm is based on the work of
/// Dr Martin Porter on his snowball project<para/>
/// refer to http://snowball.sourceforge.net/french/stemmer.html
/// (French stemming algorithm) for details
/// </summary>
- /// @deprecated Use <see cref="Tartarus.Snowball.Ext.FrenchStemmer"/> instead,
+ /// @deprecated Use <see cref="Tartarus.Snowball.Ext.FrenchStemmer"/> instead,
/// which has the same functionality. This filter will be removed in Lucene 4.0
[Obsolete("Use FrenchStemmer instead, which has the same functionality.")]
Review Comment:
We should change the message here to include the namespace as it is above, otherwise it is ambiguous because both classes have the same name.
##########
src/Lucene.Net.Analysis.Common/Analysis/De/GermanStemmer.cs:
##########
@@ -167,7 +167,7 @@ private static void RemoveParticleDenotion(StringBuilder buffer) // LUCENENET: C
{
if (buffer.ToString(c, 4).Equals("gege", StringComparison.Ordinal))
{
- buffer.Remove(c, (c + 2) - c);
+ buffer.Delete(c, (c + 2) - c);
Review Comment:
Since we don't need to calculate the end index, this can be simplified to:
```
buffer.Remove(c, 2); // LUCENENET: (c + 2) - c == 2
```
--
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-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Use J2N methods instead of StringBuilder.Remove, #664 [lucenenet]
Posted by "NightOwl888 (via GitHub)" <gi...@apache.org>.
NightOwl888 commented on code in PR #1132:
URL: https://github.com/apache/lucenenet/pull/1132#discussion_r4370756048
##########
src/Lucene.Net.Analysis.Common/Analysis/De/GermanStemmer.cs:
##########
@@ -167,7 +167,7 @@ private static void RemoveParticleDenotion(StringBuilder buffer) // LUCENENET: C
{
if (buffer.ToString(c, 4).Equals("gege", StringComparison.Ordinal))
{
- buffer.Remove(c, (c + 2) - c);
+ buffer.Delete(c, (c + 2) - c);
Review Comment:
Since we don't need to calculate the end index, this can be simplified to:
```
buffer.Delete(c, 2); // LUCENENET: (c + 2) - c == 2
```
##########
src/Lucene.Net.Analysis.Common/Analysis/De/GermanStemmer.cs:
##########
@@ -102,32 +102,32 @@ private void Strip(StringBuilder buffer)
{
if ((buffer.Length + substCount > 5) && buffer.ToString(buffer.Length - 2, buffer.Length - (buffer.Length - 2)).Equals("nd", StringComparison.Ordinal))
{
- buffer.Remove(buffer.Length - 2, buffer.Length - (buffer.Length - 2));
+ buffer.Delete(buffer.Length - 2, buffer.Length - (buffer.Length - 2));
}
else if ((buffer.Length + substCount > 4) && buffer.ToString(buffer.Length - 2, buffer.Length - (buffer.Length - 2)).Equals("em", StringComparison.Ordinal))
{
- buffer.Remove(buffer.Length - 2, buffer.Length - (buffer.Length - 2));
+ buffer.Delete(buffer.Length - 2, buffer.Length - (buffer.Length - 2));
}
else if ((buffer.Length + substCount > 4) && buffer.ToString(buffer.Length - 2, buffer.Length - (buffer.Length - 2)).Equals("er", StringComparison.Ordinal))
{
- buffer.Remove(buffer.Length - 2, buffer.Length - (buffer.Length - 2));
+ buffer.Delete(buffer.Length - 2, buffer.Length - (buffer.Length - 2));
Review Comment:
Since we don't need to calculate the end index, this can be simplified to:
```
buffer.Delete(buffer.Length - 2, 2); // LUCENENET: buffer.Length - (buffer.Length - 2) == 2
```
--
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-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org