tags:

views:

2910

answers:

6

I have the following code in one of my methods:

foreach (var s in vars)
{
    foreach (var type in statusList)
    {
        if (type.Id == s)
        {
            Add(new NameValuePair(type.Id, type.Text));
            break;
        }
    }
}

This seems sort of ineffective to me, and I was wondering if there was a way to substitute at least one of the foreaches wih a LINQ query. Any suggestions?

EDIT: vars is an array of strings and the Add method adds an item to a CSLA NameValueList.

+2  A: 

Something like this:

foreach(var s in vars) {
    var type = statusList.FirstOrDefault(t => t.Id == s);
    if (type != null)
        Add(new NameValuePair(type.Id, type.Text));
}

Or if vars supports ForEach method, this would work too (however I recommend against overLINQifying):

vars.ForEach(s => {
    var type = statusList.FirstOrDefault(t => t.Id == s);
    if (type != null)
       Add(new NameValuePair(type.Id, type.Text));
});

I assumed type is an instance of a reference type.

Mehrdad Afshari
beat me too it :-)
Dead account
I can't see how that would compile - your use of "type" on the third line won't work, because the variable isn't declared anywhere. The lambda expression in the "Any" call doesn't persist.
Jon Skeet
Jon, Thanks for noticing! I'm still wondering how I made that idiotic mistake.
Mehrdad Afshari
+8  A: 

Basically:

var types =
    from s in vars
    let type = (
        from tp in statusList
        where tp.Id == s ).FirstOrDefault()
    where type != null
    select new NameValuePair(type.Id, type.Text)
Keith
+1 for noticing that it would only add one type per "s" :)
Jon Skeet
+1  A: 

Bart de Smet has an implementation of an extension ForEach for IEnumerable.

Jakob Christensen
+3  A: 

EDIT: I hadn't noticed the break; before

If there could be more than one type with the relevant ID, then you need to use FirstOrDefault as per Keith's answer or my second code sample below.

EDIT: Removing "multi-from" version as it's unnecessarily inefficient assuming equality/hashcode works for whatever the type of type.Id is.

A join is probably more appropriate though:

var query = from s in vars
            join type in statusList on s equals type.Id
            select new NameValuePair(type.Id, type.Text);

foreach (var pair in query)
{
    Add(pair);
}

You might want to make an AddRange method which takes an IEnumerable<NameValuePair> at which point you could just call AddRange(query).

Alternatively, you can use a LookUp. This version makes sure it only adds one type per "s".

var lookup = types.ToLookup(type => type.Id);
foreach (var s in vars)
{
    var types = lookup[s];
    if (types != null)
    {
        var type = types.First(); // Guaranteed to be at least one entry
        Add(new NameValuePair(type.Id, type.Text));
    }
}

The benefit of this is that it only goes through the list of types once to build up a dictionary, basically.

Jon Skeet
The break implies only the first match where 'type.Id = s', your solution matches all cases in the inner enumerable.
leppie
Hence the sentence starting with "if there could be..." :)
Jon Skeet
Yeah, I shoulda scrolled down too before posting the same answer :) Lesson for today: read more carefully.
leppie
+2  A: 

If your Add method is constructing a list, you might also try:

IEnumarable<NamedValuePair> result = statusList.Where(type => type.Id == s).Select(new NameValuePair(type => type.Id, type.Text));
Arcturus
+1  A: 

I don't see an answer with a group join, so here's one:

var result = vars
  .GroupJoin
  (
    statusList,
    s => s,
    type => type.Id,
    (s, g) => g.Any() ? g.First() : null
  )
  .Where(type => type != null)
  .Select(type => new NameValuePair(type.Id, type.Text));
David B