31

I have an array of doubles and I want the index of the highest value. These are the solutions that I've come up with so far but I think that there must be a more elegant solution. Ideas?

double[] score = new double[] { 12.2, 13.3, 5, 17.2, 2.2, 4.5 }; int topScoreIndex = score.Select((item, indx) => new {Item = item, Index = indx}).OrderByDescending(x => x.Item).Select(x => x.Index).First(); topScoreIndex = score.Select((item, indx) => new {Item = item, Index = indx}).OrderBy(x => x.Item).Select(x => x.Index).Last(); double maxVal = score.Max(); topScoreIndex = score.Select((item, indx) => new {Item = item, Index = indx}).Where(x => x.Item == maxVal).Select(x => x.Index).Single(); 
1
  • 1
    I wonder if people are actually looking for this when searching for this question? System.Array.IndexOf(score, score.Max()) Just saw a Unity dev use the below LINQ code for this simple task and I was face palming. Commented Aug 30, 2017 at 6:04

9 Answers 9

52

Meh, why make it overcomplicated? This is the simplest way.

var indexAtMax = scores.ToList().IndexOf(scores.Max()); 

Yeah, you could make an extension method to use less memory, but unless you're dealing with huge arrays, you will never notice the difference.

Sign up to request clarification or add additional context in comments.

5 Comments

In my book this should be the accepted answer - why a dozen lines when one will do?
...and the slowest also... .About the never notice: we all notice the Windows is slow despite of the fact "it is not dealing with huge arrays"
But this is not the slowest of all the solutions posted here. Also, could you please explain what you mean by "the Windows is slow"?
I felt myself intrigued and tested the performance of PJ7's proposal with that of the extension method. The task was to find the index of the max element (standard string comparison) in a list of random strings (8 to 32 characters long). The length of the list was up to 10000000. The results were statistically the same, e.g. for 1e7 strings of length 32 the extension method needed 1400 ms while that of PJ7 needed 1530. (Tested with Stopwatch in a unit test, debug mode, Windows 10, Intel i7, 2.8 GHz, RAM 32 GB).
Another remark: the code line written exactly as quoted above will throw an exception if scores is null or InvalidOperationException if the array is empty. So kind of try/catch will be needed.
48

I suggest writing your own extension method (edited to be generic with an IComparable<T> constraint.)

public static int MaxIndex<T>(this IEnumerable<T> sequence) where T : IComparable<T> { int maxIndex = -1; T maxValue = default(T); // Immediately overwritten anyway int index = 0; foreach (T value in sequence) { if (value.CompareTo(maxValue) > 0 || maxIndex == -1) { maxIndex = index; maxValue = value; } index++; } return maxIndex; } 

Note that this returns -1 if the sequence is empty.

A word on the characteristics:

  • This works with a sequence which can only be enumerated once - this can sometimes be very important, and is generally a desirable feature IMO.
  • The memory complexity is O(1) (as opposed to O(n) for sorting)
  • The runtime complexity is O(n) (as opposed to O(n log n) for sorting)

As for whether this "is LINQ" or not: if it had been included as one of the standard LINQ query operators, would you count it as LINQ? Does it feel particularly alien or unlike other LINQ operators? If MS were to include it in .NET 4.0 as a new operator, would it be LINQ?

EDIT: If you're really, really hell-bent on using LINQ (rather than just getting an elegant solution) then here's one which is still O(n) and only evaluates the sequence once:

int maxIndex = -1; int index=0; double maxValue = 0; int urgh = sequence.Select(value => { if (maxIndex == -1 || value > maxValue) { maxIndex = index; maxValue = value; } index++; return maxIndex; }).Last(); 

It's hideous, and I don't suggest you use it at all - but it will work.

8 Comments

It is Linq to Objects, Pascal.
@Pascal: How do you define LINQ, exactly? To me, one of the nice things about LINQ is that you can add your own operators which work smoothly with the predefined ones. Editing for performance issues.
@Jon: Got it! I was out of the track. That being said. Elegant solution!
That's a great answer Jon - thanks. I tend to refer to extension methods like this LINQ but I'm guessing that I'd lose a semantic argument if that's what it came down to.
@JonSkeet Inspired by your criticism, I just modified it as if ((value!= null && value.CompareTo(maxValue) > 0) || maxIndex == -1) That should do it.
|
13
var scoreList = score.ToList(); int topIndex = ( from x in score orderby x select scoreList.IndexOf(x) ).Last(); 

If score wasn't an array this wouldn't be half bad...

4 Comments

I have to vote up Jon; its probably the better solution overall. This is the linq-iest way to do it without writing an extension method, tho.
I have to vote up Will. I like Jon's answer but this seems to come closer to answering the question asked. Ultimately, Guy will let us know which answer is best :)
Will - I love this answer. From a purest point of view this is probably the "correct" answer but I felt that Jon's answer was what I wanted. Thanks for taking the time to answer the question.
is it really a good solution to sort the list ( O(log(n)) ) for an operation which is O(n) ?
4

Try this one which is completely LINQ and has the best performance:

var indexAtMax = scores.Select((x, i) => new { x, i }) .Aggregate((a, a1) => a.x > a1.x ? a : a1).i; 

1 Comment

If you replace the anonymous object with a tuple, you can avoid an object allocation per element for even better performance.
2

This isn't the only Aggregate based solution, but this is really just a single line solution.

double[] score = new double[] { 12.2, 13.3, 5, 17.2, 2.2, 4.5 }; var max = score.Select((val,ix)=>new{val,ix}) .Aggregate(new{val=Double.MinValue,ix=-1},(z,last)=>z.val>=last.val?z:last); Console.WriteLine ("maximum value is {0}", max.val ); Console.WriteLine ("index of maximum value is {0}", max.ix ); 

3 Comments

You are right. I didn't know select has an overloaded version that has an index for the lambda as parameter, until i saw your answer.
Doesn't work if your list contains negative values. You could solve with val=Double.MinValue and z.val>=last.val (so you get the correct index if array contains MinValue).
@idbrii Good idea, I added those improvements to handle a larger range of the number space.
1

I had this problem today (to get the index in a users array who had highest age), and I did on this way:

var position = users.TakeWhile(u => u.Age != users.Max(x=>x.Age)).Count(); 

It was on C# class, so its noob solution, I´am sure your ones are better :)

1 Comment

you do realize that each time you compare u.Age to users.Max(x=>x.Age), you are doing another another N trip over the IEnumerable? Making it: O(n^2) complexity.
1

System.Linq.Enumerable.Select with index and System.Linq.Enumerable.Aggregate would do it in one line

public static int IndexOfMax<TSource>(this IEnumerable<TSource> source) where TSource : IComparable<TSource> => source.Select((value, idx) => (value, idx)) .Aggregate((aggr, next) => next.value.CompareTo(aggr.value) > 0 ? next : aggr).idx; 

1 Comment

Welcome to SO. Please notice, that this question is from the year 2009. The answer you provided might not be available back then. Also, please try to avoid code only answers. While the code might be a solution to the problem, it might not be understandable to everyone. Please add some explanation to it.
0

The worst possible complexity of this is O(2N) ~= O(N), but it needs to enumerate the collection two times.

 void Main() { IEnumerable<int> numbers = new int[] { 1, 2, 3, 4, 5 }; int max = numbers.Max (); int index = -1; numbers.Any (number => { index++; return number == max; }); if(index != 4) { throw new Exception("The result should have been 4, but " + index + " was found."); } "Simple test successful.".Dump(); } 

Comments

0

If you want something that looks LINQy, in that it's purely functional, then Jon Skeets' answer above can be recast as:

public static int MaxIndex<T>(this IEnumerable<T> sequence) where T : IComparable<T> { return sequence.Aggregate( new { maxIndex = -1, maxValue = default(T), thisIndex = 0 }, ((agg, value) => (value.CompareTo(agg.maxValue) > 0 || agg.maxIndex == -1) ? new {maxIndex = agg.thisIndex, maxValue = value, thisIndex = agg.thisIndex + 1} : new {maxIndex = agg.maxIndex, maxValue = agg.maxValue, thisIndex = agg.thisIndex + 1 })). maxIndex; } 

This has the same computational complexity as the other answer, but is more profligate with memory, creating an intermediate answer for each element of the enumerable.

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.