Dangers of LINQ
We love LINQ! We have used it for a lot of things on our project, and it has helped us with hitting a lot of deadlines. Unfortunately, in one instance it's been burning us for a few months now, and we didn't know it. Look at the code sample below. I've renamed the classes and a bunch of other things that might be considered confidential, but the logic is identical. Can you spot the bug? Try to before you read on to see the fix.
public class MyService
{
protected DataContext db = new DataContext();
public IList<ContentElement> CopySomething(Guid targetContainerID, List<Guid> sourceElementIDs)
{
List<ContentElement> sourceElements = db.ContentElements.Where(ce => sourceElementIDs.Contains(ce.ContentElementID)).ToList();
IEnumerable<ContentElement> newElements = sourceElements.Select((se, position) =>
new ContentElement()
{
ContentElementID = Guid.NewGuid(),
ContainerID = targetContainerID,
ContentXML = se.ContentXML == null ? null : new XElement(se.ContentXML),
FormatXML = se.FormatXML == null ? null : new XElement(se.FormatXML),
IsCopied = true,
ImageSourceID = ImageSource.Copy
}
);
sourceElements.ForEach(ce => ce.IsCopied = true);
db.ContentElements.InsertAllOnSubmit(newElements);
db.SubmitChanges();
return newElements.ToList();
}
}
Did you find it? It took me a long time to realize what was going on. Here's the fixed version.
public class MyService
{
protected DataContext db = new DataContext();
public IList<ContentElement> CopySomething(Guid targetContainerID, List<Guid> sourceElementIDs)
{
List<ContentElement> sourceElements = db.ContentElements.Where(ce => sourceElementIDs.Contains(ce.ContentElementID)).ToList();
List<ContentElement> newElements = sourceElements.Select((se, position) =>
new ContentElement()
{
ContentElementID = Guid.NewGuid(),
ContainerID = targetContainerID,
ContentXML = se.ContentXML == null ? null : new XElement(se.ContentXML),
FormatXML = se.FormatXML == null ? null : new XElement(se.FormatXML),
IsCopied = true,
ImageSourceID = ImageSource.Copy
}
).ToList();
sourceElements.ForEach(ce => ce.IsCopied = true);
db.ContentElements.InsertAllOnSubmit(newElements);
db.SubmitChanges();
return newElements;
}
}
Do you know what was broken, and why adding a .ToList() fixed it? Go ahead, guess... leave a comment if you will.
It finally hit me when I put the following command into the immediate window: newElements.First().ContentElementID
I did this a second time for some reason, and realized that the value was DIFFERENT?
Why is this? Well, it turns out that before I added the .ToList() after the select statement (and changed it to a list), it was getting requeried each time I asked for it. It was a query, not a set of actual values, after all. Thus, the write to the database was getting one set of values, and the the return was getting another set of values. The side effect of this error was, unfortunately, being hidden on the client side until recent functionality exposed it. So it sat there merrily humming along producing inconsistent data for months, and never really hurt anybody. Amazing.