Marcus
Marcus

Reputation: 5447

Is there a "prettier" way to do this?

Say I have the line

dirs.ToList().ForEach(d => item.Items.Add(MyMethod(d)));

But now I want to avoid adding to my list if the method returns null. Is there a prettier way than

foreach(var d in dirs)
{
    object obj = MyMethod(d);
    if(obj != null)
        myList.Items.Add(obj);
}

?

Upvotes: 0

Views: 147

Answers (4)

user166390
user166390

Reputation:

If it were my code, this is likely how it would look:

foreach(var name in dirs         // dunno if "name" is correct :)
      .Select(d => MyMethod(d))  // or .Select(MyMethod), depending
      .Where(n => n != null)) {
    myList.Items.Add(name);
}

Of, if AddRange is supported:

myList.Items.AddRange(
  dirs.Select(d => MyMethod(d))
      .Where(n => n != null));

However, I find nothing terrible about the foreach case posted and, in fact: foreach { if ... } is very desirable in cases.

This is because it can be trivially expanded to foreach { if ... else ... }. C# 3.0/4.0 doesn't have a terribly friendly way of dealing with partitioned lists and, in cases where the order is import, the approach of partitioning and then processing the lists separately is not a viable solution. In such cases, I still generally (but not always) use LINQ for as much of the transformation as possible:

foreach(var name in dirs
      .Select(d => MyMethod(d))) {
   if (name != null) {
     myList.Items.Add(name);
   } else {
     SingMerrySongsAndDanceAHappyLittleJig(); // or ... whatever
   }
}

I do not use ForEach for side-effects and I avoid side effects in LINQ queries. This includes exceptions! Much of my code that deals with LINQ actually returns IList<T> and not IEnumerable<T> to ensure it is "forced" in the appropriate context. Laziness if good, except when it isn't: C# is not Haskell.

Happy coding.

Upvotes: 2

svick
svick

Reputation: 244787

You can use Select() to call MyMethod() and then filter the list using Where():

var filteredDirs = dirs.Select(MyMethod)
                       .Where(d => d != null);

If item.Items supports AddRange() (or if you write it as an extension method for it), you can then do something like this:

item.Items.AddRange(filteredDirs);

If it doesn't support it (and you don't want to write an extension method fo it), foreach is probably best:

foreach (var dir in filteredDirs)
    item.Items.Add(dir);

But there is nothing wrong with simple imperative code like you posted, as others said.

Upvotes: 3

Alastair Pitts
Alastair Pitts

Reputation: 19601

dirs.Select(dir => MyMethod(dir))
    .Where(result => result != null)
    .ForEach(result => item.Items.Add(result));

would be how I would go about it.

Upvotes: 1

Ed Swangren
Ed Swangren

Reputation: 124642

I fail to see what is wrong with a simple loop. Honestly, I don't know why some people have an obsessive need to use LINQ or whatever shiny new tool/syntactical construct they have recently discovered when the simplest and most logical answer is right in front of them.

There is nothing wrong with your loop; worry about solving real problems.

Upvotes: 10

Related Questions