CODEWITHSUNDEEP
c#foreachienumerable
Andre Borges
Andre Borges

Reputation: 1432

IEnumerable - Update objects inside foreach loop

I have a really simple program that creates a bunch of objects and iterates through them to set each object's Priority property.

static void Main(string[] args)
{
    foreach (var obj in ObjectCreator.CreateObjectsWithPriorities())
        Console.WriteLine(String.Format("Object #{0} has priority {1}",
                                         obj.Id, obj.Priority));
}

class ObjectCreator
{
    public static IEnumerable<ObjectWithPriority> CreateObjectsWithPriorities()
    {
        var objs = new[] { 1, 2, 3 }.Select(i => new ObjectWithPriority() { Id = i });
        ApplyPriorities(objs);
        return objs;
    }

    static void ApplyPriorities(IEnumerable<ObjectWithPriority> objs)
    {
        foreach (var obj in objs)
        {
            obj.Priority = obj.Id * 10;
            Console.WriteLine(String.Format("Set priority of object #{0} to {1}", obj.Id, obj.Priority));
        }
    }
}

class ObjectWithPriority
{
    public int Id { get; set; }
    public int Priority { get; set; }
}

I'm expecting the IEnumerable in the Main method to contain objects with modified priorities. However, all of them have the default value 0. Here is the log:

Set priority of object #1 to 10 
Set priority of object #2 to 20 
Set priority of object #3 to 30 
Object #1 has priority 0 
Object #2 has priority 0 
Object #3 has priority 0

What is the reason for suche behavior and what should I change here in order to get my priorities working?

Upvotes: 12

Views: 4489

Answers (3)

holdenmcgrohen
holdenmcgrohen

Reputation: 1063

To better understand what is happening in your program, you should think of this expression

var objs = new[] { 1, 2, 3 }.Select(i => new ObjectWithPriority() { Id = i });

as a query, not as a sequence/list/collection of objects.

But it is obvious from your code that in this particular program you don't need a query. You need a collection that has a finite number of objects and that returns the same objects every time you loop through it using foreach.

So a decent thing to do would be to use ICollection<ObjectWithPriority> instead of IEnumerable<ObjectWithPriority>. This would better represent the program's logic and help avoid some mistakes/misinterpretations like the one you stumbled upon.

The code could be modified as follows:

public static ICollection<ObjectWithPriority> CreateObjectsWithPriorities()
{
    IEnumerable<ObjectWithPriority> queryThatProducesObjectsWithPriorities = new[] { 1, 2, 3 }.Select(i => new ObjectWithPriority() { Id = i }); // just for clarification
    ICollection<ObjectWithPriority> objectsWithPriorities = queryThatProducesObjectsWithPriorities.ToList();
    ApplyPriorities(objectsWithPriorities);
    return objectsWithPriorities;
}

static void ApplyPriorities(ICollection<ObjectWithPriority> objs)
{
    foreach (var obj in objs)
    {
        obj.Priority = obj.Id * 10;
        Console.WriteLine(String.Format("Set priority of object #{0} to {1}", obj.Id, obj.Priority));
    }
}

Upvotes: 2

Yuval Itzchakov
Yuval Itzchakov

Reputation: 149528

When you do this:

var objs = new[] { 1, 2, 3 }.Select(i => new ObjectWithPriority() { Id = i });

You're simply creating a lazily evaluated iterator, this doesn't allocate an array/list to store the ObjectWithPriorty you project. Each time you enumerate the iterator, it will iterate the values again and project an ObjectWithPriority for each iteration, but will discard them.

What you want to do is materialize the query before you pass it, so later you'll actually modifying the already allocated list/array. This can be achieved using Enumerable.ToList or Enumerable.ToArray:

public static IEnumerable<ObjectWithPriority> CreateObjectsWithPriorities()
{
    var objs = new[] { 1, 2, 3 }.Select(i => new ObjectWithPriority() { Id = i })
                                .ToList();
    ApplyPriorities(objs);
    return objs;
}

You could additional use Enumerable.Range instead of allocating a fixed size array, which will lazily project the numbers as requested:

var objs = Enumerable.Range(1, 3).Select(i => new ObjectWithPriority { Id = i })
                                 .ToList();

Upvotes: 17

Mike Rhosoft
Mike Rhosoft

Reputation: 1

In Addition to the Answer of Yuval Itzchakov:

If you want to lazy-load the priority of your objects you could:

Define your ApplyPriorities() Method just for one object and use it in the select-Method OR add a delegate to your ObjectWithPriority class wich calculates the Priority like shown in the code below:

class ObjectWithPriority
{
    public int Id { get; set; }

    private int? priority;
    public int Priority {
        get
        {
            return (priority.HasValue ? priority.Value : (priority = PriorityProvider(this)).Value);

        }

        set { priority = value; }
    }

    Func<ObjectWithPriority, int> PriorityProvider { get; set; }

    public ObjectWithPriority(Func<ObjectWithPriority, int> priorityProvider = null)
    {
        PriorityProvider = priorityProvider ?? (obj => 10 * obj.Id);
    }
}

Upvotes: 0

Related Questions