91

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.

Arda Xi
  • 2,616
  • 3
  • 19
  • 24
  • 2
    Is your dictionary supposed to be or is that backwards? – Stephan May 10 '10 at 19:19
  • 1
    You're right, it's . Corrected. – Arda Xi May 10 '10 at 19:25
  • why do you have a .Select after where? I'm not that saavy with LINQ, just curious – PositiveGuy Apr 03 '13 at 06:23
  • 1
    @CoffeeAddict the .Select allows him to do "projection" Here, he is converting the KeyValuePair into just a Key. He could have left this part out and just wrote `.First().Key;` to get the key instead. – dss539 Apr 03 '13 at 15:55
  • @dss539 Ah, a bit late, but you're right. That would be more efficient. – Arda Xi Apr 16 '13 at 22:39
  • @ArdaXi Actually, it might only save a few MSIL instructions. I was just trying to explain to CoffeeAddict what your code was doing. It's perfectly fine the way it is. :) – dss539 Apr 17 '13 at 13:54

10 Answers10

165

edit: .NET 6 introduced a new method
var max = results.MaxBy(kvp => kvp.Value).Key;

You should probably use that if you can.


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;
dss539
  • 6,804
  • 2
  • 34
  • 64
  • 3
    +1 dss539: I had an itch in my brain, like there should have been some way to do it with LINQ. Nice! – JMarsch May 11 '10 at 15:32
  • very helpful, I was just wondering how do we know explicitly that `l` is the return value, just for understanding since c# tends to have very clear syntax here I understood something like that was happening but its a little bit obscure. (I came due to a copilot suggestion it gave me the aggregate function and I am glad I looked up since I am going to use `MinBy()`) – Barreto May 10 '23 at 10:33
  • @Barreto lambda syntax offers a shortcut for returning values. It's a bit like python. The value of the last operation in the function is returned to the caller automatically. `x => 2*x`is the same as `x => return 2*x` – dss539 May 11 '23 at 02:29
  • well i didnt mean the return by lambda i meant the assignment the return to `l`, for example I use the predicates often, `x` being a object on a collection, like a foreach in my understanding. what i dont understand is how/when is the return value of that expression is assigned to the next round variable. is it specific for the `Aggregate` function could you use it in Find function? this is just hypothetical to explain the question, but I will look for documentation referring to this – Barreto May 11 '23 at 12:36
  • @Barreto yes that is just a behavior of the Aggregate function. You have to read the description of any function to learn what it does with the results returned by your lambda – dss539 May 11 '23 at 21:36
45

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.

WhoIsRich
  • 4,053
  • 1
  • 33
  • 19
13

You can sort dictionary by using:

  • OrderByDescending (for max value)
    or
  • OrderBy (for find min value).
    Then get first element.
    It also help when you need find second max/min element

First max/min value:
Get dictionary key by max value:

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

Get dictionary key by min value:

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

Second max/min value:
Get dictionary key by second max value:

double max = 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;
eusataf
  • 807
  • 1
  • 12
  • 24
Geograph
  • 2,274
  • 23
  • 23
12

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;

JMarsch
  • 21,484
  • 15
  • 77
  • 125
  • 1
    I know it leads to the same result but I have found this to be more readable: `var max = default(KeyValuePair);` – ChaosPandion May 10 '10 at 19:59
  • 2
    You are right; the OP had an O(2n) algorithm using see. See my answer for a O(n) using LINQ. – dss539 May 10 '10 at 20:11
  • 4
    +1 for reducing the iterations. You should probably initialize max.Value with double.MinValue to ensure you find a max even if it is a negative. – Chris Taylor May 10 '10 at 20:14
  • keep it simple :) classic search algoritims are always available. No need to think it will be hard. – Davut Gürbüz May 22 '12 at 08:23
7

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;
adrianbanks
  • 81,306
  • 22
  • 176
  • 206
Mark Byers
  • 811,555
  • 193
  • 1,581
  • 1,452
3

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;

kamyker
  • 311
  • 2
  • 6
3

Check These out:

result.Where(x=>x.Value==result.Values.Max()).Select(x=>x.Key).ToList()
Dharman
  • 30,962
  • 25
  • 85
  • 135
1

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");
    }
Zar Shardan
  • 5,675
  • 2
  • 39
  • 37
0

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);
  }
});
Jake Drew
  • 2,230
  • 23
  • 29
  • I'm pretty sure this example has a race condition. Between the time you compare the max value against the current, and swap them, another thread could have done the same thing and already swapped a better value into maxValue that will then be clobbered by the current thread's worse value. – dss539 May 27 '15 at 15:14
  • I have updated the answer with a more robust solution which I think addresses the potential race condition. – Jake Drew May 27 '15 at 19:50
  • I think you're right. That's how I was thinking of resolving the race. I do wonder if a read-write lock would have better performance. +1 for the update – dss539 May 28 '15 at 00:39
-4

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

ist_lion
  • 3,149
  • 9
  • 43
  • 73