Raymond Holmboe
Raymond Holmboe

Reputation: 2171

Nested foreach to change list items

I need to change items in a list using nested for/foreach loops. Problem is that I could not get it working using LINQ with or without dot notation. The traditional way worked and goes like this:

foreach (MapObjectLayer mapObjectLayer in map.Objects)
{
    foreach (MapObject mapObject in mapObjectLayer.MapObjects)
    {
        for (int i = 0; i < mapObject.Points.Count; i++)
        {
            mapObject.Points[i] = new Vector2(
                mapObject.Points[i].X * map.Scale,
                mapObject.Points[i].Y * map.Scale);
        }
    }
}

using LINQ, this failed:

var test = (from mol in map.Objects
           from mo in mol.MapObjects
           from p in mo.Points
           select p).ToList();

for (int i = 0; i < test.Count(); i++)
{
    test[i] = new Vector2(
        test[i].X * map.Scale,
        test[i].Y * map.Scale);
}

and this failed:

map.Objects.ForEach(l => l.MapObjects.ForEach(t => t.Points.ForEach(p => p = p * map.Scale)));

If I could get the dot notation variant working I would be very happy, but I do not have a clue on why it fails. Using the debugger it is obvious by examining the Points list that the vectors did not get multiplied using the two LINQ variants.

Update: Vector2 is a struct

Update: Here is two more one-liners that I found (working ones):

map.Objects.SelectMany(m => m.MapObjects).ToList().ForEach(o => o.Points = o.Points.Select(p => p * 2).ToList());
map.Objects.ForEach(l => l.MapObjects.ForEach(t => t.Points = t.Points.Select(p => p * 2).ToList()));

Upvotes: 0

Views: 2814

Answers (2)

Curiosa Globunznik
Curiosa Globunznik

Reputation: 3205

You can use Linq as well to collect items (in fact Resharper will offer a refactoring from nested foreach to linq), but you have to take care what you collect and what you update.

The initial linq query syntax example collects copies of Points into a new List and then replaces each element of this new list with a new instance of Vector2. Even if Vector2 would have been a referency type only the new list would change rather than the original map.Objects-substructure.

It would work the way you want it if

  • you work with reference rather than value types and
  • you'd assign the properties of the instead-of-Vector2 items:

    test[i].X *= map.Scale

Upvotes: 1

Kendall Frey
Kendall Frey

Reputation: 44326

The regular foreach is the best way. LINQ is designed for querying. You can do it in one line, but it won't be elegant or readable. Here is how:

map.Objects.ForEach(l => l.MapObjects.ForEach(t => Enumerable.Range(0, t.Points.Count).ToList().ForEach(i => t.Points[i] *= map.Scale)));

The reason that your version didn't work is because Vector2 is a value type. In the query, it's value is copied, so when you do p => p = ... you are assigning to a copy of the variable.

Use the original code. LINQ is not a replacement for loops.

Upvotes: 2

Related Questions