Good way to get the key of the highest value of a Dictionary in C#

I'm trying to get the key of the maximum value in the Dictionary<string, double> results.

This is what I have so far:

double max = results.Max(kvp => kvp.Value);
return results.Where(kvp => kvp.Value == max).Select(kvp => kvp.Key).First();

However, since this seems a little inefficient, I was wondering whether there was a better way to do this.

I think this is the most readable O(n) answer using standard LINQ.

var max = results.Aggregate((l, r) => l.Value > r.Value ? l : r).Key;

edit: explanation for CoffeeAddict

Aggregate is the LINQ name for the commonly known functional concept Fold

It loops over each element of the set and applies whatever function you provide. Here, the function I provide is a comparison function that returns the bigger value. While looping, Aggregate remembers the return result from the last time it called my function. It feeds this into my comparison function as variable l. The variable r is the currently selected element.

So after aggregate has looped over the entire set, it returns the result from the very last time it called my comparison function. Then I read the .Key member from it because I know it's a dictionary entry

Here is a different way to look at it [I don't guarantee that this compiles ;) ]

var l = results[0];
for(int i=1; i<results.Count(); ++i)
{
    var r = results[i];
    if(r.Value > l.Value)
        l = r;        
}
var max = l.Key;

After reading various suggestions, I decided to benchmark them and share the results.

The code tested:

// TEST 1

for (int i = 0; i < 999999; i++)
{
  KeyValuePair<GameMove, int> bestMove1 = possibleMoves.First();
  foreach (KeyValuePair<GameMove, int> move in possibleMoves)
  {
    if (move.Value > bestMove1.Value) bestMove1 = move;
  }
}

// TEST 2

for (int i = 0; i < 999999; i++)
{
  KeyValuePair<GameMove, int> bestMove2 = possibleMoves.Aggregate((a, b) => a.Value > b.Value ? a : b);
}

// TEST 3

for (int i = 0; i < 999999; i++)
{
  KeyValuePair<GameMove, int> bestMove3 = (from move in possibleMoves orderby move.Value descending select move).First();
}

// TEST 4

for (int i = 0; i < 999999; i++)
{
  KeyValuePair<GameMove, int> bestMove4 = possibleMoves.OrderByDescending(entry => entry.Value).First();
}

The results:

Average Seconds Test 1 = 2.6
Average Seconds Test 2 = 4.4
Average Seconds Test 3 = 11.2
Average Seconds Test 4 = 11.2

This is just to give an idea of their relative performance.

If your optimizing 'foreach' is fastest, but LINQ is compact and flexible.

Maybe this isn't a good use for LINQ. I see 2 full scans of the dictionary using the LINQ solution (1 to get the max, then another to find the kvp to return the string.

You could do it in 1 pass with an "old fashioned" foreach:

KeyValuePair<string, double> max = new KeyValuePair<string, double>();
foreach (var kvp in results)
{
if (kvp.Value > max.Value)
max = kvp;
}
return max.Key;

You can sort dictionary by using OrderBy (for find min value) or OrderByDescending (for max value) then get first element. It also help when you need find second max/min element

Get dictionary key by max value:

double min = results.OrderByDescending(x => x.Value).First().Key;

Get dictionary key by min value:

double min = results.OrderBy(x => x.Value).First().Key;

Get dictionary key by second max value:

double min = results.OrderByDescending(x => x.Value).Skip(1).First().Key;

Get dictionary key by second min value:

double min = results.OrderBy(x => x.Value).Skip(1).First().Key;

This is a fast method. It is O(n), which is optimal. The only problem I see is that it iterates over the dictionary twice instead of just once.

You can do it iterating over the dictionary once by using MaxBy from morelinq.

results.MaxBy(kvp => kvp.Value).Key;

Little extension method:

public static KeyValuePair<K, V> GetMaxValuePair<K,V>(this Dictionary<K, V> source)
	where V : IComparable
{
	KeyValuePair<K, V> maxPair = source.First();
	foreach (KeyValuePair<K, V> pair in source)
	{
		if (pair.Value.CompareTo(maxPair.Value) > 0)
			maxPair = pair;
	}
	return maxPair;
}

Then:

int keyOfMax = myDictionary.GetMaxValuePair().Key;

Check These out:

result.Where(x=>x.Value==result.Values.Max()).Select(x=>x.Key).ToList()

How about doing it in parallel using Interlocked.Exchange for thread safety :) Keep in mind that Interlocked.Exchange will only work with a reference type.(i.e. a struct or key value pair (unless wrapped in a class) will not work to hold the max value.

Here's an example from my own code:

//Parallel O(n) solution for finding max kvp in a dictionary...
ClassificationResult maxValue = new ClassificationResult(-1,-1,double.MinValue);
Parallel.ForEach(pTotals, pTotal =>
{
    if(pTotal.Value > maxValue.score)
    {
        Interlocked.Exchange(ref maxValue, new                
            ClassificationResult(mhSet.sequenceId,pTotal.Key,pTotal.Value)); 
    }
});

EDIT (Updated code to avoid possible race condition above):

Here's a more robust pattern which also shows selecting a min value in parallel. I think this addresses the concerns mentioned in the comments below regarding a possible race condition:

int minVal = int.MaxValue;
Parallel.ForEach(dictionary.Values, curVal =>
{
  int oldVal = Volatile.Read(ref minVal);
  //val can equal anything but the oldVal
  int val = ~oldVal;

  //Keep trying the atomic update until we are sure that either:
  //1. CompareExchange successfully changed the value.
  //2. Another thread has updated minVal with a smaller number than curVal.
  //   (in the case of #2, the update is no longer needed)
  while (oldval > curVal && oldval != val)
  {
    val = oldval;
    oldval = Interlocked.CompareExchange(ref minVal, curVal, oldval);
  }
});

My version based off the current Enumerable.Max implementation with an optional comparer:

    public static TSource MaxValue<TSource, TConversionResult>(this IEnumerable<TSource> source, Func<TSource, TConversionResult> function, IComparer<TConversionResult> comparer = null)
    {
        comparer = comparer ?? Comparer<TConversionResult>.Default;
        if (source == null) throw new ArgumentNullException(nameof(source));

        TSource max = default;
        TConversionResult maxFx = default;
        if ( (object)maxFx == null) //nullable stuff
        {
            foreach (var x in source)
            {
                var fx = function(x);
                if (fx == null || (maxFx != null && comparer.Compare(fx, maxFx) <= 0)) continue;
                maxFx = fx;
                max = x;
            }
            return max;
        }

        //valuetypes
        var notFirst = false;
        foreach (var x in source) 
        {
            var fx = function(x);
            if (notFirst)
            {
                if (comparer.Compare(fx, maxFx) <= 0) continue;
                maxFx = fx;
                max = x;
            }
            else
            {
                maxFx = fx;
                max = x;
                notFirst = true;
            }
        }
        if (notFirst)
            return max;
        throw new InvalidOperationException("Sequence contains no elements");
    }

Example usage:

    class Wrapper
    {
        public int Value { get; set; }    
    }

    [TestMethod]
    public void TestMaxValue()
    {
        var dictionary = new Dictionary<string, Wrapper>();
        for (var i = 0; i < 19; i++)
        {
            dictionary[$"s:{i}"] = new Wrapper{Value = (i % 10) * 10 } ;
        }

        var m = dictionary.Keys.MaxValue(x => dictionary[x].Value);
        Assert.AreEqual(m, "s:9");
    }

I think using the standard LINQ Libraries this is as fast as you can go.