Can this be refactored into nicey nice LINQ?

Question

I have an IList of type Breadcrumb which is just a lightweight class that has NavigationTitle, NavigationUrl and IsCurrent properties. It is cached on the webserver. I have a method that builds out the current breadcrumb trail up until the first Breadcrumb that has IsCurrent set to true... using the code below. Its very ugly and definitely a quick dirtbag willie solution, but I was curious, can this be easily refactored into LINQ?

IList<Breadcrumb> crumbs = new List<Breadcrumb>();
bool foundCurrent = false;
for (int a = 0; a < cachedCrumbs.Count; a++)
{
    crumbs.Add(crumbs[a]);
    if (foundCurrent)
    {
      break;
    }
    foundCurrent = (crumbs[a + 1] != null && ((Breadcrumb)crumbs[a + 1]).IsCurrent);
}

Answer 1

First of all, that code doesn't work. I'm gonna guess that some of those places where you used "crumbs" you meant "cachedCrumbs". If so, the code can be reduced to:

IList<Breadcrumb> crumbs = new List<Breadcrumb>();
for (int a = 0; a < cachedCrumbs.Count; a++)
{
    crumbs.Add(cachedCrumbs[a]);
    if (cachedCrumbs[a] != null && cachedCrumbs[a].IsCurrent)
    {
          break;
    }
}

Answer 2

I'm typing this as I think, so that it shows a train of thought as well as just an answer.

• Your source is just cachedCrumbs
• You want to add the first crumb which does have IsCurrent set, but nothing afterwards
• TakeWhile sounds like the way to go, but getting the "previous value had IsCurrent" is a bit of a pain
• We can use a closure to effectively keep a variable determining whether the last value had IsCurrent set
• We can do a somewhat "no-op" select to keep the TakeWhile separate from the working out of whether to keep going

So, we end up with:

bool foundCurrent = false;

var crumbs = cachedCrumbs.TakeWhile(crumb => !foundCurrent)
                         .Select(crumb => { 
                                 foundCurrent = crumb == null || !crumb.IsCurrent; 
                                 return crumb; });

I haven't tried this, but I think it should work... there might be a simpler way though.

EDIT: I'd argue that actually a straight foreach loop is simpler in this case. Having said that, you could write another extension method which acted like TakeWhile except it also returned the element which caused the condition to fail. Then it would be as simple as:

var crumbs = cachedCrumbs.NewMethod(crumb => crumb == null || !crumb.IsCurrent);

(I can't think of a decent name for the method at the moment, hence NewMethod !)

Answer 3

An alternative answer based on James Curran's - this can certainly be improved using a foreach statement:

IList<Breadcrumb> crumbs = new List<BreadCrumb>();
foreach (Breadcrumb crumb in cachedCrumbs)
{
    crumbs.Add(crumb);
    if (crumb != null && crumb.IsCurrent)
    {
        break;
    }
}

Answer 4

How about...

// find the current item
var currentItem = cachedCrumbs.First(c => c.IsCurrent);
var currentIdx = cachedCrumbs.IndexOf(currentItem);

// get all items upto current item
var crumbs = cachedCrumbs.Take(currentIdx + 2);

And you can turn this into a TakeUpto method which takes all items upto the one which matched the predicates you supply.

How about:

public static IEnumerable<T> TakeUpto<T>(this IList<T> theList, Func<T, bool> predicate)
{
    var targetItem = theList.First(predicate);
    var targetIdx = theList.IndexOf(targetItem);

    return theList.Take(targetIdx + 2);
}

Then you could use it this way:

var crumbs = cachedCrumbs.TakeUpto(c => c.IsCurrent);

Much cleaner!

Havn't check the nulls and off-by-one cases and IList/IEnumerable differences, but you should get the idea.

Answer 5

This answer is an alternative implementation of chakrit's TakeUpTo:

public static IEnumerable<T> TakeUpto<T>(this IEnumerable<T> theList, Func<T, bool> predicate)
{
    foreach (T element in theList)
    {
        yield return element;
        if (predicate(element))
        {
            break;
        }
    }
}

This only iterates through the list once, which could be relevant in various cases. (Suppose the upstream sequence is the result of an OrderBy clause - you really don't want it to have to sort the results multiple times for no good reason.)

It also allows any IEnumerable<T> as the source, which makes it more flexible.

One of the wonderful things about LINQ is the multiple ways of achieving the same goal.