tags:

views:

156

answers:

3

I've got a simple class defined as:

public class IndexEntry
{
   public bool HighScore { get; set; }
   public List<IndexEntry> SubEntries { get; set; }
   //Other properties, etc...
}

I now need to search through a List to find the one item that has its HighScore Property set to true. Since it isn't a flat list, but a Hierarchy which can be an unknown number of levels deep and since the item I'm looking for might be contained in any one of the SubEnties lists, I can't do a simple Lambda like this:

var foundHighScore = myList.FirstOrDefault(IE => IE.HighScore == true);

Here's the code I have. I know it's ugly (at least it seems that way to me). It works, but is slow as sin on an even remotely large list and I'm sure there must be a better way.

private IndexEntry GetHighScoreEntry(IEnumerable<IndexEntry> entryList)
{
    IndexEntry result = null;
    IndexEntry recursiveResult = null;
    foreach (IndexEntry currentEntry in entryList)
    {
        if (currentEntry.HighScore)
        {
            result = currentEntry;
            break;  //Don't need to look anymore, we found our highscore.;
        }
        else
        {
            if ((currentEntry.SubEntries == null) || (currentEntry.SubEntries.Count < 1))
            {
                continue;
            }
            else
            {
                recursiveResult = GetHighScoreEntry(currentEntry.SubEntries);
                if (recursiveResult == null)
                    continue;
                    result = recursiveResult;
                break;
            }
        }
    }
    return result;
}

I've got believe that there's a better way using a slightly more complex lambda or with LINQ to clean up this code and make it more performant.

Thanks in advance for your help.

A: 

You could significantly narrow your search down with a lambda expression something like:

var foundHighScore = myList.FirstOrDefault(IE => IE.HighScore or (IE.SubEntries != null && IE.SubEntries.Any(IES => IES.HighScore));

var indexEntry = foundHighScore;
if (!indexEntry.HighScore)
{
    indexEntry = indexEntry.SubEntries.FirstOrDefault(IE => IE.HighScore);
}

// do something with indexEntry

Update

Actually the first solution won't traverse through properly. I don't think there is a lambda-only solution to this, you will have to do some form of recursive function. Off the top of my head the following would work, how it would cope performance wise I am unsure:

public IndexEntry FindHighScore(List<IndexEntry> entries)
{
    var highScore = GetHighScore(entries);
    if (highScore == null)
    {
        // give me only entries that contain sub-entries
        var entriesWithSub = entries.Where(e => e.SubEntries != null);
        foreach (var e in entriesWithSub)
        {
            highScore = FindHighScore(e.SubEntries);
            if (highScore != null)
                return highScore;
        }
    }
    return highScore;
}

private IndexEntry GetHighScore(List<IndexEntry> entries)
{
    return entries.FirstOrDefault(IE => IE.HighScore);
}
James
James. Thanks. I was actually trying myself to use a combination of your and _rusty's solutions to get what I need. I'm going to try this out and report back on performance.
Steve Brouillard
To be honest both answer's aren't really that far away from eachother so it's really whichever one gives you the best performance. Even benchmark them against your current solution.
James
That's the plan.
Steve Brouillard
+2  A: 

Recursion is your friend here.

public IndexEntry FindHighScore(IEnumerable<IndexEntry> entries)
{
  foreach (IndexEntry entry in entries)
  {
    IndexEntry highScore = FindHighScore(entry);
    if (highScore != null)
    {
      return highScore;
    }
  }
  return null;
}

private IndexEntry FindHighScore(IndexEntry entry)
{
  return entry.HighScore ? entry : FindHighScore(entry.SubEntries);
}
_rusty
(Of course this could use some null checking in the right places but that exercise is left for the reader ;))
_rusty
+1  A: 

All the solutions posted so far are specialized - they are not generic or general, and thus, the next time you have a hierarchical list, you'll have to code up a new solution. Yuck.

Here's a general, generic solution that will work for all your hierarchical needs:

public static IEnumerable<T> Flatten<T>(this IEnumerable<T> sequence, Func<T, IEnumerable<T>> childFetcher)
{
    var itemsToYield = new Queue<T>(sequence);
    while (itemsToYield.Count > 0)
    {
        var item = itemsToYield.Dequeue();
        yield return item;

        var children = childFetcher(item);
        if (children != null)
        { 
            foreach (var child in children) itemsToYield.Enqueue(child);
        }
    }
}

Here's how you'd use it:

myList.Flatten(i => i.SubEntries).FirstOrDefault(i => i.HighScore);

Easy as cheese.

This extension method can be used to turn any hierarchical data into a flat list, which can them be searched using LINQ.

Another great thing about this solution is that is uses lazy evaluation, thus it only does as much work as the called demands. For example, in the above code, Flatten will stop churning out items as soon as a HighScore is found.

This solution also avoids recursion, which can be a costly operation for deeply nested hierarchies, avoiding the many stack allocations that recursive solutions incur.

Judah Himango
Judah. I like your overall idea, but am not comfortable enough in C# yet to troubleshoot an issue I'm having with your code. When I attempt to comple your code I get the following error - The body of 'IEnumerableExtensions.Flatten<T>(System.Collections.Generic.IEnumerable<T>, System.Func<T,System.Collections.Generic.IEnumerable<T>>)' cannot be an iterator block because 'void' is not an iterator interface type.
Steve Brouillard
This error suggests to me that you need to return a type (IEnumerable<T> perhaps?) and then work with that item?
Steve Brouillard
@Steve, you are correct his method must have a return type of IEnumerable<T> for the yield to work.
James
Trying it now and testing performance.
Steve Brouillard
Woops, yeah, it should return IEnumerable<T>. I've fixed this in the post.
Judah Himango
So, just a quick comment for those who are interested. This version seems to give me about a 20% performance boost over using some type of recursion.
Steve Brouillard
Glad to hear it, Steve.
Judah Himango