C# Paradigms: Side effects on Lists

Question

I am trying to evolve my understanding of side effects and how they should be controlled and applied.

In the following List of flights, I want to set a property of each flight satisfying a conditions:

IEnumerable<FlightResults> fResults = getResultsFromProvider();

//Set all non-stop flights description
fResults.Where(flight => flight.NonStop)
        .Select(flight => flight.Description = "Fly Direct!");

In this expression, I have a side effect on my list. From my limited knowledge I know for ex. "LINQ is used for queries only" and "There are only a few operations to lists and assigning or setting values is not one of them" and "lists should be immutable".

• What is wrong with my LINQ statement above and how should it be changed?
• Where can I get more information on the fundamental paradigms on the scenario I have described above?

Drain

Use a "foreach" loop to induce a side effect. Selection is for projection, not for updating. That's why its called "Select" and not "Update". – Nanettenani

related question here https://mcmap.net/q/376757/-linq-side-effects – Civilized

Answer 1

Your LINQ code does not "directly" violate the guidelines you mention, because you are not modifying the list itself; you are just modifying some property on the contents of the list.

However, the main objection that drives these guidelines remains: you should not be modifying data with LINQ (also, you are abusing Select to perform your side effects).

Not modifying any data can be justified pretty easily. Consider this snippet:

fResults.Where(flight => flight.NonStop)  

Do you see where this is modifying the flight properties? Neither will many maintenance programmers, since they will stop reading after the Where -- the code that follows is obviously free of side effects since this is a query, right?

[Nitpick: Certainly, seeing a query whose return value is not retained is a dead giveaway that the query does have side effects or that the code should have been removed; in any case, that "something is wrong". But it's so much easier to say that when there are only 2 lines of code to look at instead of pages upon pages.]

As a correct solution, I would recommend this:

foreach (var x in fResults.Where(flight => flight.NonStop))
{
    x.Description = "Fly Direct!";
}

Pretty easy to both write and read.

Hofstetter

Answer 2

You have two ways of achieving it the LINQ way:

1. explicit foreach loop

   foreach(Flight f in fResults.Where(flight => flight.NonStop))
     f.Description = "Fly Direct!";
   

2. with a ForEach operator, made for the side effects:

   fResults.Where(flight => flight.NonStop)
           .ForEach(flight => flight.Description = "Fly Direct!");
   

The first way is quite heavy for such a simple task, the second way should only be used with very short bodies.

Now, you might ask yourself why there isn't a ForEach operator in the LINQ stack. It's quite simple - LINQ is supposed to be a functional way of expressing query operations, which especially means that none of the operators are supposed to have side effects. The design team decided against adding a ForEach operator to the stack because the only usage is its side effect.

A usual implementation of the ForEach operator would be like this:

public static class EnumerableExtension
{
  public static void ForEach<T> (this IEnumerable<T> source, Action<T> action)
  {
    if(source == null)
      throw new ArgumentNullException("source");

    foreach(T obj in source)
      action(obj);

  }
}

Gilletta

oh, I didn't know that. Interesting. – Gilletta

@leppie actually the Query won't work as expected. At the end of the OP's code block the values are not changed. But yes, the statement in the OP will compile/run. – Adduct

@Femaref: it is sometimes handy to do in-place assignments. Eg (for a property with lazy initializer) return x ?? (x = new Foo()); . But I would not use it in a call to Select :) – Cheque

@Rangoric: Should have added an iteration would have been needed, eg calling Count(). – Cheque

@Rangoric: The query works as expected in the LINQ stack, but probably not how you expect intuitively (if you don't know the inner mechanics of LINQ). – Gilletta

@Gilletta I mean as expected by the OP for it to have it just change the values. – Adduct

@Gilletta So is the usage of the .ForEach(...) an acceptable implementation given the standard that lists should not have side effects? What is wrong with the .Select(...) project it to new values as in my example? – Drain

Lists (or in the LINQ sense: sequences) can't have side effects, only the operators can. As the ForEach operator explicitly states that is has side effects, yes, it is acceptable. In your case, Select doesn't only project into something (project meaning taking the input, transforming it in a way with only taking into account the original input), it also changes it, which is against the functional paradigm. – Gilletta

Answer 3

One problem with that approach is that it won't work at all. The query is lazy, which means that it won't execute the code in the Select until you actually read something from the query, and you never do that.

You could come around that by adding .ToList() at the end of the query, but the code is still using side effects and throwing away the actual result. You should use the result to do the update instead:

//Set all non-stop flights description
foreach (var flight in fResults.Where(flight => flight.NonStop)) {  
  flight.Description = "Fly Direct!";
}

Labaw

Yes I do realize that. Can I however use .Select(...) as in the example and .ToList() just before I want to display the results? – Drain

@Pierre: You don't need ToList at all if you just want to display a result. The result from the code in your question is pretty useless to display as it's just a list of strings that are all the same. – Labaw

Answer 4

I like using foreach when I'm actually changing something. Something like

foreach (var flight in fResults.Where(f => f.NonStop))
{
  flight.Description = "Fly Direct!";
}

and so does Eric Lippert in his article about why LINQ does not have a ForEach helper method.

But we can go a bit deeper here. I am philosophically opposed to providing such a method, for two reasons.

The first reason is that doing so violates the functional programming principles that all the other sequence operators are based upon. Clearly the sole purpose of a call to this method is to cause side effects.

Carder

Good article that explains some core ideas regarding my question thanks. – Drain

Answer 5

There is nothing wrong with it perse, except that you need to iterate it somehow, like calling Count() on it.

From a 'style' perspective it is not good. One would not expect an iterator to mutate a list value/property.

IMO the following would be better:

foreach (var x in fResults.Where(flight => flight.NonStop))
{
  x.Description = "Fly Direct!";
}

The intent is much clearer to the reader or maintainer of the code.

Cheque

Answer 6

You should break that up into two blocks of code, one for the retrieval and one for setting the value:

var nonStopFlights = fResults.Where(f => f.NonStop);

foreach(var flight in nonStopFlights)
    flight.Description = "Fly Direct!";

Or, if you really hate the look of foreach you could try:

var nonStopFlights = fResults.Where(f => f.NonStop).ToList();

// ForEach is a method on List that is acceptable to make modifications inside.
nonStopFlights.ForEach(f => f.Description = "Fly Direct!");

Giffy

+1 I was about to post an answer about the whole "Never actually changing what it needs to" piece. – Adduct