64

I have two tables: Transactions and TransactionAgents. TransactionAgents has a foreign key to Transactions called TransactionID. Pretty standard.

I also have this code:

BrokerManagerDataContext db = new BrokerManagerDataContext();

var transactions = from t in db.Transactions
                   where t.SellingPrice != 0 
                   select t;

var taAgents = from ta in db.TransactionAgents
               select ta;

foreach (var transaction in transactions)
{
    foreach(var agent in taAgents)
    {
        agent.AgentCommission = ((transaction.CommissionPercent / 100) * (agent.CommissionPercent / 100) * transaction.SellingPrice) - agent.BrokerageSplit;
    } 
}

dataGridView1.DataSource = taAgents;

Basically, a TransactionAgent has a property/column named AgentCommission, which is null for all TransactionAgents in my database.

My goal is to perform the math you see in the foreach(var agent in taAgents) to patch up the value for each agent so that it isn't null.

Oddly, when I run this code and break-point on agent.AgentCommission = (formula) it shows the value is being calculated for AgentCommissision and the object is being updated but after it displays in my datagrid (used only for testing), it does not show the value it calculated.

So, to me, it seems that the Property isn't being permanently set on the object. What's more, If I persist this newly updated object back to the database with an update, I doubt the calculated AgentCommission will be set there.

Without having my table set up the same way, is there anyone that can look at the code and see why I am not retaining the property's value?

dtlvd
  • 457
  • 1
  • 8
  • 21
Isaiah Nelson
  • 2,450
  • 4
  • 34
  • 53

5 Answers5

105

IEnumerable<T>s do not guarantee that updated values will persist across enumerations. For instance, a List will return the same set of objects on every iteration, so if you update a property, it will be saved across iterations. However, many other implementations of IEnumerables return a new set of objects each time, so any changes made will not persist.

If you need to store and update the results, pull the IEnumerable<T> down to a List<T> using .ToList() or project it into a new IEnumerable<T> using .Select() with the changes applied.

To specifically apply that to your code, it would look like this:

var transactions = (from t in db.Transactions
                    where t.SellingPrice != 0 
                    select t).ToList();

var taAgents = (from ta in db.TransactionAgents
                select ta).ToList();

foreach (var transaction in transactions)
{
    foreach(var agent in taAgents)
    {
        agent.AgentCommission = ((transaction.CommissionPercent / 100) * (agent.CommissionPercent / 100) * transaction.SellingPrice) - agent.BrokerageSplit;
    } 
}

dataGridView1.DataSource = taAgents;

eouw0o83hf
  • 9,438
  • 5
  • 53
  • 75
  • 1
    Oh. Thats nice to know. So, I need to .ToList() my enumerable before I foreach over them? – Isaiah Nelson Feb 01 '12 at 22:20
  • Yes, that way the values will be stored locally in the List instead of temporarily in a disposed-of object. – eouw0o83hf Feb 01 '12 at 22:22
  • 1
    -1 This answer is simply *wrong*. You definitely can change object properties that are accessed via `IEnumerable`. The `List` itself implements `IEnumerable`.... – ken2k Mar 08 '13 at 13:04
  • 4
    On another look, you're right - I guess the more correct description is an `IEnumerable` does not *guarantee* that updating its values will persists across enumerations. – eouw0o83hf Mar 08 '13 at 14:57
  • 3
    I did go crazy with this :) Thanks so much. – QMaster Feb 01 '16 at 09:41
  • @ken2k Imagine code `from t ... select new MyDTO { X = x, Y = y }` every time you enumerate such a sequence you are creating new class that doesn't see previous changes. I have such a bug in production and after that we required that all methods that return `IEnumerable` must call `ToArray` or `ToList` to prevent lazy evaluation. – csharpfolk Oct 27 '16 at 17:33
  • @eouw0o83hf what is strange here is that we are enumerating EnityFramework `DbSet`s. Generally EF should cache objects loaded from DB (and I may assume that change tracking is also enabled) and every iteration should return the same objects. Am I wrong here? EDIT: I found my answer http://stackoverflow.com/questions/21707970/why-does-entity-framework-6-x-not-cache-results – csharpfolk Oct 27 '16 at 17:35
4

Specifically, the problem is that each time you access the IEnumerable, it enumerates over the collection. In this case, the collection is a call to the database. In the first part, you're getting the values from the database and updating them. In the second part, you're getting the values from the database again and setting that as the datasource (or, pedantically, you're setting the enumerator as the datasource, and then that is getting the values from the database).

Use .ToList() or similar to keep the results in memory, and access the same collection every time.

Kyle W
  • 3,702
  • 20
  • 32
  • Wow. So the problem is much worse than eow0o83hf described. I understand that its sort of "readonly" but the enumerable is essentially wiping out the values as the collection is read. – Isaiah Nelson Feb 01 '12 at 22:22
  • It's not so much that it's wiping out the values, but that it's reading from a different collection. (In simple (but not technically correct) terms, it creates a new "collection" every time you enumerate it) – Kyle W Feb 01 '12 at 22:24
3

Assuming you are using LINQ to SQL, if EnableObjectTracking is false, then the objects will be constructed new every time the query is run. Otherwise, you would be getting the same object instances each time and your changes would survive. However, like others have shown, instead of having the query execute multiple times, cache the results in a list. Not only will you get what you want working, you'll have fewer database round trips.

Matt Warren
  • 1,956
  • 14
  • 15
1

I found that I had to locate the item in the list that I wanted to modify, extract the copy, modify the copy (by incrementing its count property), remove the original from the list and add the modified copy. var x = stats.Where(d => d.word == s).FirstOrDefault(); var statCount = stats.IndexOf(x); x.count++; stats.RemoveAt(statCount); stats.Add(x);

David
  • 79
  • 1
  • 1
  • 1
    This is very inefficient and bad coding practice. You really should not use LINQ for this, especially with lists. just use the normal operator[]. – JHBonarius Sep 26 '18 at 08:58
0

It is helpful to rewrite your LINQ expression using lambdas so that we can consider the code in more explicit terms.

//Original code from question
var taAgents = from ta in db.TransactionAgents
               select ta;
//Rewritten to explicitly call attention to what Select() is actually doing
var taAgents = db.TransactionAgents.Select(ta => new TransactionAgents(/*database row's data*/)});

In the rewritten code, we can clearly see that Select() is constructing a new object based on each row returned from the database. What's more, this object construction occurs every time the IEnumerable taAgents is iterated through.

So, explained more concretely, if there are 5 TransactionAgents rows in the database, in the following example, the TransactionAgents() constructor is called a total of 10 times.

// Assume there are 5 rows in the TransactionAgents table
var taAgents = from ta in db.TransactionAgents
               select ta;

//foreach will iterate through the IEnumerable, thus calling the TransactionAgents() constructor 5 times
foreach(var ta in taAgents)
{
  Console.WriteLine($"first iteration through taAgents - element {ta}");
}
// these first 5 TransactionAgents objects are now out of scope and are destroyed by the GC


//foreach will iterate through the IEnumerable, thus calling the TransactionAgents() constructor 5 MORE times
foreach(var ta in taAgents)
{
  Console.WriteLine($"second iteration through taAgents - element {ta}");
}

// these second 5 TransactionAgents objects are now out of scope and are destroyed by the GC

As we can see, all 10 of our TransactionAgents objects were created by the lambda in our Select() method, and do not exist outside of the scope of the foreach statement.

MattEvansDev
  • 615
  • 7
  • 15