API proposal: Comparison by key

[jnm2]

Before

You have to write out the selector twice:

names.Sort((first, second) => string.Compare(
    first.Identifier.GetIdentifierText(),
    second.Identifier.GetIdentifierText(),
    StringComparer.OrdinalIgnoreCase));

(Comparer.Sequence is #33873)

statements.OrderBy(GetNameSegments, Comparer.Sequence(
    Comparer.Create((first, second) => string.Compare(
        first.Identifier.GetIdentifierText(),
        second.Identifier.GetIdentifierText(),
        StringComparer.OrdinalIgnoreCase)))

After

// Proposal 1
names.Sort(Comparer.By(
    (SimpleNameSyntax name) => name.Identifier.GetIdentifierText(),
    StringComparer.OrdinalIgnoreCase));

// Proposal 2
names.Sort(Comparer<SimpleNameSyntax>.By(
    name => name.Identifier.GetIdentifierText(),
    StringComparer.OrdinalIgnoreCase))

// Proposal 1
statements.OrderBy(GetNameSegments, Comparer.Sequence(Comparer.By(
    (SimpleNameSyntax name) => name.Identifier.GetIdentifierText(),
    StringComparer.OrdinalIgnoreCase));

// Proposal 2
statements.OrderBy(GetNameSegments, Comparer.Sequence(Comparer<SimpleNameSyntax>.By(
    name => name.Identifier.GetIdentifierText(),
    StringComparer.OrdinalIgnoreCase))

Proposal 1

namespace System.Collections
{
    public sealed class Comparer : IComparer, ISerializable
    {
+       public static IComparer<T> By<T, TKey>(
+           Func<T, TKey> keySelector,
+           IComparer<TKey>? keyComparer = null);

        // No other static members are declared by Comparer
    }
}

Proposal 2

namespace System.Collections.Generic
{
    public abstract partial class Comparer<T> : IComparer, IComparer<T>
    {
+       public static IComparer<T> By<TKey>(
+           Func<T, TKey> keySelector,
+           IComparer<TKey>? keyComparer = null);

        // Other static members declared by Comparer:
        public static Comparer<T> Create(Comparison<T> comparison);
    }
}

Draft implementation

Click to expand

private sealed class KeyComparer<T, TKey> : IComparer<T>
{
    private readonly Func<T, TKey> keySelector;
    private readonly IComparer<TKey> keyComparer;

    public KeyComparer(Func<T, TKey> keySelector, IComparer<TKey> keyComparer)
    {
        this.keySelector = keySelector;
        this.keyComparer = keyComparer;
    }

    public int Compare([AllowNull] T x, [AllowNull] T y)
    {
        return keyComparer.Compare(
            keySelector.Invoke(x!),
            keySelector.Invoke(y!));
    }
}

[svick]

I definitely like this idea, though I think it becomes much more valuable if you add the ability to compare multiple keys. That would mean instead of:

people.Sort((x, y) =>
{
    int result = x.LastName.CompareTo(y.LastName);
    
    if (result == 0)
        result = x.FirstName.CompareTo(y.FirstName);
        
    if (result == 0)
        result = x.MiddleName.CompareTo(y.MiddleName);

    return result;
});

you could write something like:

people.Sort(KeyComparer<Person>.OrderBy(p => p.LastName)
                               .ThenBy(p => p.FirstName)
                               .ThenBy(p => p.MiddleName));

A NuGet package that implements this already exists, though, to my dismay, it's not very popular.

[jnm2]

@svick One thing I've been favoring even with LINQ instead of ThenBy is to select a tuple:

people.Sort(Comparer<Person>.By(p => (p.LastName, p.FirstName, p.MiddleName)));

The only thing this doesn't let you do is specify a comparer for each tuple element, but that's a proposal I have on my list to file separately from this because I want it without selectors too.

[ghost]

Tagging subscribers to this area: @eiriktsarpalis, @jeffhandley
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Before

You have to write out the selector twice:

names.Sort((first, second) => string.Compare(
    first.Identifier.GetIdentifierText(),
    second.Identifier.GetIdentifierText(),
    StringComparer.OrdinalIgnoreCase));

(Comparer.Sequence is #33873)

statements.OrderBy(GetNameSegments, Comparer.Sequence(
    Comparer.Create((first, second) => string.Compare(
        first.Identifier.GetIdentifierText(),
        second.Identifier.GetIdentifierText(),
        StringComparer.OrdinalIgnoreCase)))

After

// Proposal 1
names.Sort(Comparer.By(
    (SimpleNameSyntax name) => name.Identifier.GetIdentifierText(),
    StringComparer.OrdinalIgnoreCase));

// Proposal 2
names.Sort(Comparer<SimpleNameSyntax>.By(
    name => name.Identifier.GetIdentifierText(),
    StringComparer.OrdinalIgnoreCase))

// Proposal 1
statements.OrderBy(GetNameSegments, Comparer.Sequence(Comparer.By(
    (SimpleNameSyntax name) => name.Identifier.GetIdentifierText(),
    StringComparer.OrdinalIgnoreCase));

// Proposal 2
statements.OrderBy(GetNameSegments, Comparer.Sequence(Comparer<SimpleNameSyntax>.By(
    name => name.Identifier.GetIdentifierText(),
    StringComparer.OrdinalIgnoreCase))

Proposal 1

namespace System.Collections
{
    public sealed class Comparer : IComparer, ISerializable
    {
+       public static IComparer<T> By<T, TKey>(
+           Func<T, TKey> keySelector,
+           IComparer<TKey>? keyComparer = null);

        // No other static members are declared by Comparer
    }
}

Proposal 2

namespace System.Collections.Generic
{
    public abstract partial class Comparer<T> : IComparer, IComparer<T>
    {
+       public static IComparer<T> By<TKey>(
+           Func<T, TKey> keySelector,
+           IComparer<TKey>? keyComparer = null);

        // Other static members declared by Comparer:
        public static Comparer<T> Create(Comparison<T> comparison);
    }
}

Draft implementation

Click to expand

private sealed class KeyComparer<T, TKey> : IComparer<T>
{
    private readonly Func<T, TKey> keySelector;
    private readonly IComparer<TKey> keyComparer;

    public KeyComparer(Func<T, TKey> keySelector, IComparer<TKey> keyComparer)
    {
        this.keySelector = keySelector;
        this.keyComparer = keyComparer;
    }

    public int Compare([AllowNull] T x, [AllowNull] T y)
    {
        return keyComparer.Compare(
            keySelector.Invoke(x!),
            keySelector.Invoke(y!));
    }
}

Author:	jnm2
Assignees:	-
Labels:	api-suggestion, area-System.Collections, area-System.Runtime
Milestone:	Future

[eiriktsarpalis]

I would probably prefer proposal 2, since the untyped Comparer type lives in the System.Collections namespace.

[lbmaian]

A similar API for EqualityComparer would also be a nice counterpart to this proposal. Just replace IComparer with IEqualityComparer and have an internal KeyEqualityComparer class implement the latter.

[eiriktsarpalis]

FWIW this feels like it could be an overload for Comparer.Create, e.g.

public class Comparer<T>
{
    public static IComparer<T> Create<TKey>(Func<T, TKey> keySelector);
    public static IComparer<T> Create<TKey>(Func<T, TKey> keySelector, IComparer<TKey>? keyComparer);
}

[eiriktsarpalis]

I think we might need to submit an API proposal that merges this with #33873 so that we can land on a holistic design.

[jnm2]

@eiriktsarpalis I also want to propose a value tuple comparer so that it's possible to e.g. specify OrdinalIgnoreCase for Dictionary<(string A, int B), Foo>. Should that be part of the holistic design? I can write it up if so.

ValueTupleComparer.Create(StringComparer.OrdinalIgnoreCase, EqualityComparer<int>.Default) or similar, so that both IComparer<T> and IEqualityComparer<T> can be implemented maybe.

[eiriktsarpalis]

Go for it. I would recommend creating a fresh issue using the API proposal template so we can start from scratch. Please also check if there are other related issues in the backlog so we can consider including them in the design.

[eiriktsarpalis]

@jnm2 there are a few IEqualityComparer<T> requests as well lurking in the backlog. Would it make sense to merge everything in one big Comparer/EqualityComparer factories proposal? For example there's #44796 and #19644.

[jnm2]

@eiriktsarpalis I'm not sure, what's been done before that's similar?

[eiriktsarpalis]

I would probably look at the static APIs in the Comparer<T> and EqualityComparer<T> classes.