MaxBy inconsistent results with tie-breaks

[martingbrown]

Description

When all the items in the list have a null key MaxBy returns the last item in the list. When they all have a key value but the values are the same the first item is returned. To me that seems inconsistent.

Reproduction Steps

using System;
using System.Linq;
					
public class Program
{
	public static void Main()
	{
		var l1 = new Tuple<string, string>[]
		{
			new Tuple<string, string>("", "A"),
			new Tuple<string, string>("", "B")
		};
		
		var l2 = new Tuple<string, string>[]
		{
			new Tuple<string, string>(null, "A"),
			new Tuple<string, string>(null, "B")
		};
		
		var r1 = l1.MaxBy(t => t.Item1);
		var r2 = l2.MaxBy(t => t.Item1);
		
		Console.WriteLine(r1.Item2);
		Console.WriteLine(r2.Item2);
	}
}

Expected behavior

The Reproduction code should print AA or BB.

Actual behavior

The reproduction code prints AB

Regression?

I think this is new in .Net 6.

Known Workarounds

Don't use lists with all the keys the same.

Configuration

I got this in .Net 6 RC2. I don't think the OS or CPU are going to make any difference.

Other information

No response

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[ghost]

Tagging subscribers to this area: @dotnet/area-system-linq
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Description

When all the items in the list have a null key MaxBy returns the last item in the list. When they all have a key value but the values are the same the first item is returned. To me that seems inconsistent.

Reproduction Steps

using System;
using System.Linq;
					
public class Program
{
	public static void Main()
	{
		var l1 = new Tuple<string, string>[]
		{
			new Tuple<string, string>("", "A"),
			new Tuple<string, string>("", "B")
		};
		
		var l2 = new Tuple<string, string>[]
		{
			new Tuple<string, string>(null, "A"),
			new Tuple<string, string>(null, "B")
		};
		
		var r1 = l1.MaxBy(t => t.Item1);
		var r2 = l2.MaxBy(t => t.Item1);
		
		Console.WriteLine(r1.Item2);
		Console.WriteLine(r2.Item2);
	}
}

Expected behavior

The Reproduction code should print AA or BB.

Actual behavior

The reproduction code prints AB

Regression?

I think this is new in .Net 6.

Known Workarounds

Don't use lists with all the keys the same.

Configuration

I got this in .Net 6 RC2. I don't think the OS or CPU are going to make any difference.

Other information

No response

Author:	martingbrown
Assignees:	-
Labels:	area-System.Linq, untriaged
Milestone:	-

[eiriktsarpalis]

Hi @martingbrown, this is by-design behavior intended to preserve compatibility with null element handling in the existing Max and Min methods: null keys have special meaning and are always treated as bottom elements regardless of what comparer is being passed by the user. As a side-effect of that behavior is that the methods will always return the last element if every key in a sequence is null.

That being said, whether the Max method is returning the first or last null has no observable difference on the user's output. This is not the case with MaxBy so I can see how this can take users by surprise. Note however that the Max* method documentation provides no guarantees on what element will be returned in the event of a tie, so I believe this behavior to be well-within specification. We can consider refining the behavior in a future release.

As a workaround -assuming order of selection is important- I would recommend avoiding null keys since they are always treated bottom elements regardless of comparer. In your case this would mean writing:

var r2 = l2.MaxBy(t => t.Item1 ?? "");