NightOwl888 commented on code in PR #1128:
URL: https://github.com/apache/lucenenet/pull/1128#discussion_r1955567281
##########
src/Lucene.Net/Support/ConcurrentHashSet.cs:
##########
@@ -42,7 +44,7 @@ namespace Lucene.Net.Support
/// concurrently from multiple threads.
/// </remarks>
[DebuggerDisplay("Count = {Count}")]
- internal class ConcurrentHashSet<T> : ISet<T>, IReadOnlyCollection<T>,
ICollection<T>
+ internal class ConcurrentHashSet<T> : ISet<T>, IReadOnlyCollection<T>
Review Comment:
Microsoft always lists all of the interfaces at the top level of
collections, even if they are inherited from others in the list. I think it is
mainly for documentation purposes, but it is also helpful to see which
interfaces are supported without drilling down.
Technically, we should support `IReadOnlySet<T>` also, but being that it
wasn't added until `net5.0`, it would require conditional compilation. It would
still be helpful to do in case this is ever moved to J2N.
##########
src/Lucene.Net/Support/ConcurrentHashSet.cs:
##########
@@ -265,7 +267,7 @@ public ConcurrentHashSet(int concurrencyLevel, int
capacity, IEqualityComparer<T
{
}
- private ConcurrentHashSet(int concurrencyLevel, int capacity, bool
growLockArray, IEqualityComparer<T> comparer)
+ private ConcurrentHashSet(int concurrencyLevel, int capacity, bool
growLockArray, IEqualityComparer<T>? comparer)
Review Comment:
The default comparer should be
`J2N.Collections.Generic.EqualityComparer<T>`, which fixes comparisons for
string, double, and float to match Java. It doesn't matter for any of our
current use cases, but it will help to future-proof this so it acts more like
the J2N collections by default.
--
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
