Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add string keyed dictionary ReadOnlySpan<char> lookup support #31942

Open
TylerBrinkley opened this issue Aug 24, 2018 · 37 comments

Comments

@TylerBrinkley
Copy link
Collaborator

commented Aug 24, 2018

Moved from corefxlab#2438 as the proposed changes could not be implemented in a separate library.

Motivation

I would like to perform dictionary lookups using a ReadOnlySpan<char> value on a string keyed dictionary without having to allocate a string.

There are two different design approaches proposed, add extension methods to Dictionary<string, TValue> or add a new Type .

Extension Method Proposed API

 namespace System.Collections.Generic {
+    public static class DictionaryExtensions {
+        public static bool ContainsKey<TValue>(this Dictionary<string, TValue> dictionary, ReadOnlySpan<char> key);
+        public static bool Remove<TValue>(this Dictionary<string, TValue> dictionary, ReadOnlySpan<char> key);
+        public static bool TryGetValue<TValue>(this Dictionary<string, TValue> dictionary, ReadOnlySpan<char> key, out TValue value);
+        public static T GetValue<TValue>(this Dictionary<string, TValue> dictionary, ReadOnlySpan<char> key);
+    }
 }
 namespace System {
      public abstract class StringComparer : IComparer<string>, IEqualityComparer<string>, IComparer, IEqualityComparer {
+        public virtual bool Equals(ReadOnlySpan<char> x, ReadOnlySpan<char> y);
+        public virtual int GetHashCode(ReadOnlySpan<char> obj);
      }
     public sealed class CultureAwareComparer : StringComparer {
+        public override bool Equals(ReadOnlySpan<char> x, ReadOnlySpan<char> y);
+        public override int GetHashCode(ReadOnlySpan<char> obj);
     }
     public class OrdinalComparer : StringComparer {
+        public override bool Equals(ReadOnlySpan<char> x, ReadOnlySpan<char> y);
+        public override int GetHashCode(ReadOnlySpan<char> obj);
     }
 }

Implementation Details

  • Internal details of Dictionary<TKey, TValue> would need to be exposed with the internal modifier to implement the ReadOnlySpan<char> overloads.
  • Only StringComparers that are passed into the constructor would be able to be optimized to not allocate a string.
  • The default implementation of the new virtual methods on StringComparer would convert the ReadOnlySpan<char> to a string and use the string methods instead.

New Type Proposed API

 namespace System.Collections.Specialized {
+    public class StringKeyedDictionary<TValue> : Dictionary<string, TValue> {
+        public StringKeyedDictionary();
+        public StringKeyedDictionary(StringComparer comparer);
+        public StringKeyedDictionary(int capacity);
+        public StringKeyedDictionary(int capacity, StringComparer comparer);
+        public StringKeyedDictionary(IDictionary<string, TValue> dictionary);
+        public StringKeyedDictionary(IDictionary<string, TValue> dictionary, StringComparer comparer);
+        public StringKeyedDictionary(IEnumerable<KeyValuePair<string, TValue>> collection);
+        public StringKeyedDictionary(IEnumerable<KeyValuePair<string, TValue>> collection, StringComparer comparer);
+        public new StringComparer Comparer { get; }
+        public T this[ReadOnlySpan<char> key] { get; }
+        public bool ContainsKey(ReadOnlySpan<char> key);
+        public bool Remove(ReadOnlySpan<char> key);
+        public bool TryGetValue(ReadOnlySpan<char> key, out TValue value);
+    }
 }
 namespace System {
      public abstract class StringComparer : IComparer<string>, IEqualityComparer<string>, IComparer, IEqualityComparer {
+        public virtual bool Equals(ReadOnlySpan<char> x, ReadOnlySpan<char> y);
+        public virtual int GetHashCode(ReadOnlySpan<char> obj);
      }
     public sealed class CultureAwareComparer : StringComparer {
+        public override bool Equals(ReadOnlySpan<char> x, ReadOnlySpan<char> y);
+        public override int GetHashCode(ReadOnlySpan<char> obj);
     }
     public class OrdinalComparer : StringComparer {
+        public override bool Equals(ReadOnlySpan<char> x, ReadOnlySpan<char> y);
+        public override int GetHashCode(ReadOnlySpan<char> obj);
     }
 }

Implementation Details

  • Internal details of Dictionary<TKey, TValue> would need to be exposed with the private protected modifier to implement the ReadOnlySpan<char> overloads.
  • The default implementation of the new virtual methods on StringComparer would convert the span to a string and use the string methods instead.
  • A null comparer passed to the constructor would default to StringComparer.Ordinal.

Possible Addition

While not specifically needed for this request a ReadOnlySpan<char> Compare overload should probably be added as well while we're updating StringComparer.

 namespace System {
     public abstract class StringComparer : IComparer, IEqualityComparer, IComparer<string>, IEqualityComparer<string> {
+        public virtual int Compare(ReadOnlySpan<char> x, ReadOnlySpan<char> y);
     }
     public sealed class CultureAwareComparer : StringComparer {
+        public override int Compare(ReadOnlySpan<char> x, ReadOnlySpan<char> y);
     }
     public class OrdinalComparer : StringComparer {
+        public override int Compare(ReadOnlySpan<char> x, ReadOnlySpan<char> y);
     }
 }

Open Questions

  • Should the additional ReadOnlySpan<char> Compare overload be included?
  • Should NonRandomizedStringEqualityComparer now derive from StringComparer in order to be supported?
  • Should a TryAdd overload be added as it has the possibility of not requiring to be converted to a string if an equal key is found?

Discussion Summary.

We have two different design approaches, add a new Type or add extension methods to Dictionary<string, TValue>.

Here's a comparison of the pro's and con's of the different design approaches.

Extension Methods StringKeyedDictionary
Pros No new type introduced. No performance penalty due to casting the comparer.
Existing code exposing a string keyed Dictionary needs no changes to use which is helpful when they can't easily be changed. Indexer support.
More scalable
Cons Performance penalty of casting the comparer. New type introduced meaning an additional staple of the .NET ecosystem to know.
No indexer support, must use a GetValue method instead. Existing code that exposes a string keyed Dictionary may not easily be changed to using the new type.
Dictionary internals exposed with the internal modifier. Dictionary internals exposed with the private protected modifier.
Somewhat opaque to user as to knowing the comparer must derive from StringComparer.

Updates

  • Updated namespaces back to System.Collections.Specialized.
  • Changed extension class name to DictionaryExtensions from StringKeyedDictionaryExtensions.
  • Removed proposed IStringEqualityComparer interface and instead relies on comparer deriving from StringComparer.
  • Reordered design approaches to better promote my preference. 😄
@svick

This comment has been minimized.

Copy link
Contributor

commented Aug 24, 2018

Would it be better to instead have a set of extension methods on Dictionary<string, T>? That way, the new methods could be used on any string-keyd Dictionary, not just on some special type.

@Wraith2

This comment has been minimized.

Copy link
Collaborator

commented Aug 24, 2018

Dictionary<TKey,TValue> is implemented in Coreclr and the is a high bar for new types because of library size issues. Do you intend or this implementation not to be collision resistant or do you have a method to store the key without the span somehow? convert it into a ReadOnlyMemory?

@benaadams

This comment has been minimized.

Copy link
Collaborator

commented Aug 24, 2018

Would it be better to instead have a set of extension methods on Dictionary<string, T>?

How would it perform the search (using hashcode + equals) against the current Dictionary api surface?

do you have a method to store the key without the span somehow? convert it into a ReadOnlyMemory?

ROS<char>.ToString()

@svick

This comment has been minimized.

Copy link
Contributor

commented Aug 24, 2018

@benaadams

Would it be better to instead have a set of extension methods on Dictionary<string, T>?

How would it perform the search (using hashcode + equals) against the current Dictionary api surface?

It wouldn't use the public API surface, it would use some new internal members on Dictionary. That's worse than StringKeyedDictionary, which would probably use new private protected members, but I think not by much.

@benaadams

This comment has been minimized.

Copy link
Collaborator

commented Aug 24, 2018

Ah.. so something like?

static class StringKeyedDictionaryExtensions 
{
    static bool ContainsKey<TValue>(this Dictionary<string, TValue> dictionary, ReadOnlySpan<char> key);
    static bool Remove<TValue>(this Dictionary<string, TValue> dictionary, ReadOnlySpan<char> key);
    static bool TryGetValue<TValue>(this Dictionary<string, TValue> dictionary, ReadOnlySpan<char> key, out TValue value);
}
@Wraith2

This comment has been minimized.

Copy link
Collaborator

commented Aug 24, 2018

Right, I think I missed the point then. The idea is to have the standard dictionary at the back end but methods which allow you to use ROS as the key to search for avoiding the allocation to check if something exists. So it needs access to the internals because the comparer needs to understand that it can do ROS-string comparisons which aren't currently possible.

@svick

This comment has been minimized.

Copy link
Contributor

commented Aug 24, 2018

@benaadams Yup. Plus probably an extension method version of the indexer:

public static T GetValue<TValue>(this Dictionary<string, TValue> dictionary, ReadOnlySpan<char> key);

Though that's an advantage of the StringKeyedDictionary design: it can have a real indexer.

@safern

This comment has been minimized.

Copy link
Member

commented Aug 24, 2018

Also if they're extension methods we would need to provide overrides to specify a custom Comparer that can compare ROS right? Or that would be achieved by adding the StringEqualityComparer type and interface and in the extension methods check if Comparer is IStringEqualityComparer then use the ROS overrides if not the string ones?

Also with override methods Adding values through the indexer or TryAdd/Add apis, we wouldn't be able to take advantage of the ROS comparisons right?

@benaadams

This comment has been minimized.

Copy link
Collaborator

commented Aug 25, 2018

Also if they're extension methods we would need to provide overrides to specify a custom Comparer that can compare ROS right?

Dictionary is based on IEqualityComparer<T> so not sure if it could work... (even if default interfaces implementations pulling up the slack, not sure how it could work for other types as a ROS comparison would be exposed on the interface, which probably wouldn't make sense?)

Also with override methods Adding values through the indexer or TryAdd/Add apis, we wouldn't be able to take advantage of the ROS comparisons right?

Can covert the string to a ROS at nominal cost for the comparison; if it didn't exist the ROS<char> could be turned into a string for persistent key use; so that wouldn't necessarily be a problem.

@safern

This comment has been minimized.

Copy link
Member

commented Aug 25, 2018

Dictionary is based on IEqualityComparer so not sure if it could work... (even if default interfaces implementations pulling up the slack, not sure how it could work for other types as a ROS comparison would be exposed on the interface, which probably wouldn't make sense?)

Yes, but people could just use StringEqualityComparer and since it would implement IEqualityComparer it can be passed through the constructor. However in order to use the ROS overloads if we go through the Extension methods route we would need to cast the Comparer to IStringEqualityComparer

I think if we want to use the ROS comparison features and other of its features through a custom Comparer we should not use Extension methods, unless I’m missing something we could do in a different way without overriding implementation.

@TylerBrinkley

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 25, 2018

Here's a comparison of the pro's and con's of the different design approaches.

Extension Methods StringKeyedDictionary
Pros No new type introduced. No performance penalty due to casting the comparer.
Existing code exposing a string keyed Dictionary needs no changes to use which is helpful when they can't easily be changed. Indexer support.
Cons Performance penalty of casting the comparer. New type introduced meaning an additional staple of the .NET ecosystem to know.
No indexer support, must use a GetValue method instead. Existing code that exposes a string keyed Dictionary may not easily be changed to using the new type.
Dictionary internals exposed with the internal modifier. Dictionary internals exposed with the private protected modifier.
Somewhat opaque to user as to knowing it uses IStringEqualityComparer.

Honestly, I'm kind of torn on this. Adding a new foundational Type to the ecosystem is costly in terms of what developers need to know. I'm feeling this even more now with the addition of all the types introduced with Spans and Pipelines. Additionally, I could easily see this expanding to others such as HashSet, SortedDictionary, SortedSet, OrderedDictionary & OrderedSet if they ever get implemented, and ConcurrentDictionary.

If the performance isn't an issue I now think I'd prefer the extension method route.

@safern

This comment has been minimized.

Copy link
Member

commented Aug 28, 2018

@TylerBrinkley thanks for the great summary, I just updated the issue description and included this comparison in there. Will mark as ready for review now.

@TylerBrinkley

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 28, 2018

I've updated the first post to correct formatting and make it a little easier to understand.

@karelz karelz added this to the Future milestone Aug 28, 2018
@cdmihai

This comment has been minimized.

Copy link

commented Aug 28, 2018

Overloads for ConcurrentDictionary would also be nice :)

@terrajobst

This comment has been minimized.

Copy link
Member

commented Aug 28, 2018

Video

On the surface, the extension approach looks more appealing because it would allow the lookups to work against any instance of a dictionary (at least in principle, the current design only works if the dictionary was instantiated with a comparer that implements IStringEqualityComparer).

However, this also complicates the implementation and might have performance implications if we have to expose more methods that only make sense when TKey happens to be string. Also, one could argue that in the hot paths where people would be inclined to lookup with spans that they do in fact control their dictionary and whether they need to pass in the correct comparer or use a derived type of Dictionary<,> doesn't matter.

I totally buy the scenario, but adding a new collection type (even if derived from Dictionary<,>) feels heavy handed, but maybe I'm overthinking this. Maybe just putting it in S.C.Specialized is good enough.

What do others think?

@jkotas

This comment has been minimized.

Copy link
Member

commented Aug 28, 2018

This is trying to workaround the fundamental problem that ref-like types cannot be used as keys in collections today. We will hit the same problem for all keyed collections and many ref-like types. @cdmihai mentioned that ConcurrentDictionary would be nice too. The same can be said for HashMap and other collections. And the same can be said for Utf8 strings, Spans and other ref-like types. If we take this proposal, where are we going to stop – are we going to add all combinations like StringKeyedHashMap, SpanKeyedConcurrentDictionary?

I think we need to have ref struct constraint first to make this scalable.

@TylerBrinkley

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 28, 2018

I agree scalability is an issue if we add new types which is why I'm now in favor of the extension methods route which should scale more favorably.

Even if the ref struct constraint is added I don't see how we can use that for this use case in a non-stack context such as non-ref-like class fields or static fields.

@Joe4evr

This comment has been minimized.

Copy link

commented Aug 30, 2018

This is trying to workaround the fundamental problem that ref-like types cannot be used as keys in collections today.

It's not so much using ReadOnlySpan<char> as the dictionary key, but doing a lookup with it without copy-allocating the slice it is wrapping.

@benaadams

This comment has been minimized.

Copy link
Collaborator

commented Aug 30, 2018

Is what's equatable/alternative forms; so string <=> ReadOnlySpan<char>; however what about string <=> ReadOnlySpan<byte>, Encoding.UTF8 etc

@raffree

This comment has been minimized.

Copy link

commented Aug 30, 2018

I would change NameValueCollection, making more effective version, passing allowDublicates parameter in constructor.
For dictionary i think extension method will fit completely.

@TylerBrinkley

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 30, 2018

what about string <=> ReadOnlySpan<byte>, Encoding.UTF8 etc

Another reason the extension methods route makes better sense.

What if we instead relied on the comparer to derive from StringComparer and not define an interface? That way we can easily expand support through adding virtual methods without having to define new interfaces or else wait for default interface implementation support.

@Joe4evr

This comment has been minimized.

Copy link

commented Aug 30, 2018

what about string <=> ReadOnlySpan<byte>, Encoding.UTF8 etc

What if we instead relied on the comparer to derive from StringComparer and not define an interface?

Sounds reasonable. I can already see use-cases for wanting to determine the value equality between a string and utf8string.

//using regular old UTF-16 strings as a key due to framework reasons,
//even though 99.9% of the time, it doesn't go outside UTF-8 range
private readonly Dictionary<string, Foo> _fooMap = new Dictionary<string, Foo>(StringComparer.OrdinalIgnoreCase);

//what this proposal already boils down to:
void M(ReadOnlySpan<char> sliceOfString)
{
    if (_fooMap.TryGetValue(sliceOfString, out var foo)
    {
        //....
    }
}

//but when incoming data could also be UTF-8:
void M(ReadOnlySpan<byte> sliceOfUtf8String) //or 'ROS<Char8>' if that becomes the more appropriate thing
{
    if (_fooMap.TryGetValue(sliceOfUtf8String, out var foo)
    {
        //....
    }
}
@TylerBrinkley

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 26, 2018

Really appreciated hearing the discussion of this in the .NET Design Reviews GitHub Triage.

I think this should go back to triage to decide the best design approach. Again, personally I'd much prefer the extension method route.

@TylerBrinkley TylerBrinkley changed the title Add a StringKeyedDictionary<T> type Add ReadOnlySpan<char> lookup support for string keyed dictionary Sep 26, 2018
@TylerBrinkley TylerBrinkley changed the title Add ReadOnlySpan<char> lookup support for string keyed dictionary Add string keyed dictionary ReadOnlySpan<char> lookup support Sep 26, 2018
@DaZombieKiller

This comment has been minimized.

Copy link

commented Dec 12, 2018

What if an API was added to Dictionary<TKey, TValue> that allows you to retrieve a bucket?
For example (where DictionaryBucket is a new type, mainly to ensure that it's read-only):

DictionaryBucket<TValue> Dictionary<TKey, TValue>.GetBucket(int hashCode) or
IReadOnlyList<TValue> Dictionary<TKey, TValue>.GetBucket(int hashCode)

This way, the extension methods could operate on public APIs, and would just need to iterate over the bucket and compare values.

I'm not sure how realistic this would be, and I'm absolutely certain that there are significant problems with this that I haven't considered, but I figured I'd throw the idea out there.

@Wraith2

This comment has been minimized.

Copy link
Collaborator

commented Dec 12, 2018

It would be exposing and locking implementation details of the dictionary, it would prevent reimplementing using another method if a better one were found in future.

@TylerBrinkley

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 22, 2019

As Dictionary doesn't currently support this scenario I was looking at implementing this in a custom dictionary and am having trouble getting the hashcode of the span. Are there any API's currently exposed which allows the retrieval of the hashcode of a ReadOnlySpan<char> in the Ordinal or OrdinalIgnoreCase sense without converting to a string?

@jkotas

This comment has been minimized.

Copy link
Member

commented Apr 22, 2019

Look at the APIs introduced in #31302

@TylerBrinkley

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 22, 2019

@jkotas Thanks. Since .NET Standard 2.1 is including some API's introduced for .NET Core 3.0 would including this API be considered as well?

@jkotas

This comment has been minimized.

Copy link
Member

commented Apr 22, 2019

.NET Standard 2.1 is very close to done at this point. We do not plan to add more APIs to it.

APIs introduced in .NET Core 3.0 are not included in .NET Standard 2.1 by default. Exceptions were made for a few APIs, mostly the ones required to support C# 8.0.

@TylerBrinkley

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 22, 2019

@jkotas Thanks. I guess I'll resort to multi-targeting .NET Core App as well then.

@scalablecory

This comment has been minimized.

Copy link
Member

commented Aug 5, 2019

I would prefer a generalized "proxy key lookup" support on Dictionary/etc. than specializing it for strings.

@TylerBrinkley

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 6, 2019

@scalablecory what use case would you have for a generalized "proxy key lookup"? Also, what kind of API would you propose that would address this use case given the nature of the stack-only ReadOnlySpan<char>?

@scalablecory

This comment has been minimized.

Copy link
Member

commented Aug 7, 2019

@TylerBrinkley we will need to combine a ref struct generic constraint with a new proposal for heterogeneous lookup on keyed containers.

Special-casing some support for String would be a band-aid that we can't remove later on, and I'd rather wait for language changes needed to make a proper generalized change.

@TylerBrinkley

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 14, 2019

@scalablecory Are you thinking of something like this?

public interface IAltEqualityComparer<T, TAlt> : IEqualityComparer<T> {
    int GetHashCode(TAlt value);
    bool Equals(TAlt value, T other);
}
public interface IAltRefEqualityComparer<T, TAlt> : IEqualityComparer<T> where TAlt : ref struct {
    int GetHashCode(TAlt value);
    bool Equals(TAlt value, T other);
}
public class Dictionary<TKey, TValue> {
    public bool ContainsKey<TAltKey>(TAltKey altKey);
    public bool ContainsKeyRef<TAltKey>(TAltKey altKey) where TAltKey : ref struct;
}

or this?

public delegate int GetHashCodeDelegate<TAltKey, TKey>(TAltKey altKey, IEqualityComparer<TKey> comparer);
public delegate bool EqualsDelegate<TAltKey, TKey>(TAltKey altKey, TKey key, IEqualityComparer<TKey> comparer);
public delegate int GetHashCodeDelegateRef<TAltKey, TKey>(TAltKey altKey, IEqualityComparer<TKey> comparer) where TAltKey : ref struct;
public delegate bool EqualsDelegateRef<TAltKey, TKey>(TAltKey altKey, TKey key, IEqualityComparer<TKey> comparer) where TAltKey : ref struct;

public class Dictionary<TKey, TValue> {
    public bool ContainsKey<TAltKey>(TAltKey altKey, GetHashCodeDelegate<TAltKey, TKey> getHashCode, EqualsDelegate<TAltKey, TKey> equals);
    public bool ContainsKey<TAltKey>(TAltKey altKey, GetHashCodeDelegateRef<TAltKey, TKey> getHashCode, EqualsDelegateRef<TAltKey, TKey> equals) where TAltKey : ref struct;
}
@scalablecory

This comment has been minimized.

Copy link
Member

commented Aug 19, 2019

@TylerBrinkley I'm imagining a hybrid of the two.

C# changes - ref struct constraint

Need a better name for this, and a C# proposal. Maybe a CLR proposal too...

The constraint would enforce that values of the type are only used in ways which are compatible with ref struct types.

So, you can pass in either a normal type or a ref struct, but it can't be stored in a non-stack field, can't be captured or used in async, etc.

Library changes

public interface IEqualityComparer<T, TAlt> where TAlt: ref struct
{
    int GetHashCode(TAlt value);
    bool Equals(TAlt value, T other);
}

class StringComparer : IEqualityComparer<string, ReadOnlySpan<char>>
{
    public int GetHashCode(ReadOnlySpan<char> value);
    public bool Equals(ReadOnlySpan<char> value, string other);
}
// also Utf8String / Char8

class Dictionary<TKey, TValue>
{
    public bool ContainsKey<TAltKey>(TAltKey key, IEqualityComparer<TKey, TAltKey> comparer) where TAltKey: ref struct;
    public bool TryGetValue<TAltKey>(TAltKey key, IEqualityComparer<TKey, TAltKey> comparer, out TValue value) where TAltKey: ref struct;
    public bool Remove<TAltKey>(TAltKey key, IEqualityComparer<TKey, TAltKey> comparer) where TAltKey: ref struct;
    public bool Remove<TAltKey>(TAltKey key, IEqualityComparer<TKey, TAltKey> comparer, out TValue value) where TAltKey: ref struct;
}
// plus ConcurrentDictionary

class HashSet<T>
{
    public bool Contains<TAlt>(TAlt item, IEqualityComparer<T, TAlt> comparer) where TAlt: ref struct;
    public bool Remove<TAlt>(TAlt item, IEqualityComparer<T, TAlt> comparer) where TAlt: ref struct;
    public bool TryGetValue<TAlt>(TAlt item, IEqualityComparer<T, TAlt> comparer, out T actualValue) where TAlt: ref struct;
}

// IComparer, SortedDictionary, SortedSet? Not sure if worth it.

Risks

Passing in an incompatible comparer might cause these to return an incorrect value. Usage of these will only happen in more advanced scenarios, so I think this is acceptable.

@TylerBrinkley

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 19, 2019

I really don't like passing in the comparer in the method call due to the incompatible comparer issues and general usability. Is there a reason you don't suggest having the methods just try to type cast the collection's comparer?

@scalablecory

This comment has been minimized.

Copy link
Member

commented Aug 19, 2019

I really don't like passing in the comparer in the method call due to the incompatible comparer issues and general usability. Is there a reason you don't suggest having the methods just try to type cast the collection's comparer?

I agree, it's not good for usability and I don't like it. Mainly looking for feedback :).

I'd really like to find ways to avoid a cast, because:
a) adds a cast in what will be used in perf-sensitive code. (may or may not be significant depending on type/data -- need to measure)
b) moves usage errors from compile-time to run-time.

To look at an existing implementation, C++ has something in between these two approaches:

  • Everything is baked into the comparer.
  • A default comparer supporting this (std::less<void>) uses ADL and thus has the same usability concerns.
  • ... but sidesteps those issues involved with a cast because it resolves all of this at compile time rather than run time.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.