0

Hi have a list of object and each object has another list items respectively. I want to update a property in a sub list after checking a condition. I have tried a linq query but it doesn't update the property. Please help me

int priceBookId = 1;
foreach (var store in stores)
{
    store.SelectedBooks.Where(d => d.BookID == priceBookId ).Select(x => {x.IsPriceBook = true; return x; });
}

Each stores has selected book list and each book has its own properties. Here what I want is to update IsPriceBook property when selected book is a pricebook

Incredible
  • 3,495
  • 8
  • 49
  • 77
user2837480
  • 349
  • 2
  • 18
  • 2
    Code as shown should update `IsPriceBook` if match found. Please provide real [MCVE] that shows this code not working. Side note: discussion whether updating objects as part of LINQ query is good idea or not is completely opinion based and outside of scope of SO. Personally I'd not allow anyone around me to do so as it is very confusing and not going to work with something like LINQ-to-SQL (which is likely will be your case when you finally [edit] post with complete example) – Alexei Levenkov Jan 29 '18 at 06:42
  • @FaizanRabbani it exactly what I mean in my comment - you never expected attempt to change object's properties in `.Select` call... and as result read `x.IsPriceBook = true` as `x.IsPriceBook == true`... – Alexei Levenkov Jan 29 '18 at 06:44
  • @AlexeiLevenkov Yes, after reading your comment, I deleted mine. – FaizanHussainRabbani Jan 29 '18 at 06:45
  • Please check this answer: https://stackoverflow.com/questions/5317459/query-and-updating-a-property-in-a-collection-using-linq/22445009 – Incredible Jan 29 '18 at 06:46
  • Actually my comment is partly wrong and code does not work... for exact reasons (delayed execution) covered in post linked by Ity Tyagi. – Alexei Levenkov Jan 29 '18 at 08:51

3 Answers3

1

The code in the post demonstrates why doing updates of objects in .Select (or other LINQ methods) is bad idea:

  • no one ever expect modification to happen, do not expect result to persist or variations of those two
  • delayed/lazy execution of LINQ queries makes it very hard to see what is happening. Code will work fine while debugging and looking at results then fail while running on its own.

Problem with original code - Where and Select are lazily evaluated and hence nothing requests enumeration of the result no evaluation actually happens. To fix - forcing iteration would work. Use .ToList() or .All(...) as shown in Query and updating a property in a collection using LINQ.

foreach (var store in stores)
{
  store.SelectedBooks
   .Where(d => d.BookID == priceBookId )
   .Select(x => {x.IsPriceBook = true; return 42; }) // return anything
   .ToList(); // force iteration with `ToList`
}

You can even remove outer foreach 

stores.SelectMany(r =>r.SelectedBooks)
   .Where(d => d.BookID == priceBookId )
   .All(x => {x.IsPriceBook = true; return true; }); // force iteration with All

Indeed fixing code that way will be consider as "hack" and really you should first select item and that act on them:

foreach (var book in 
     stores.SelectMany(store => store.SelectedBooks)
          .Where(d => d.BookID == priceBookId ))
{
  book.IsPriceBook = true;
}
Alexei Levenkov
  • 98,904
  • 14
  • 127
  • 179
0

select will create new object with updated properties, so if you make like:

var updatedProperties = store.SelectedBooks.Where(d => d.BookID == priceBookId ).Select(x => {x.IsPriceBook = true; return x; });

you will get the result in updatedProperties variable. in your case. you can update property if you use 2 cycles, something like the following:

foreach (var store in stores)
{
    foreach (var selectedBook in store.SelectedBooks.Where(d => d.BookID == priceBookId))
    {
        selectedBook.IsPriceBook = true;
    }
}

since store.SelectedBooks has reference type, selectedBook.IsPriceBook = true; will update value by reference.

Nikolai
  • 306
  • 1
  • 10
  • This is indeed better from coding style point of view... but in no way explaining why original code does not work (and OP's code is functionally identical to one in this answer) – Alexei Levenkov Jan 29 '18 at 06:58
0

Select() does not work because that creates a projection that does nothing until you .ToList() it (then it will perform the update). But that's just using Select in the way it's not intended.

You can try ForEach, which loops through the original list, like this:

int priceBookId = 1;
foreach (var store in stores)
{
    store.SelectedBooks.Where(d => d.BookID == priceBookId).ToList().ForEach(x => x.IsPriceBook = true);
}

That's the quickest solution.

Balah
  • 2,530
  • 2
  • 16
  • 24
  • Can you please explain why you believe original code was wrong? And why your one is any better? (slower/use more copy operations *is not* generally considered "better") – Alexei Levenkov Jan 29 '18 at 06:47
  • @AlexeiLevenkov, If I understood correct, OP using the select as the final operation. But just to update `ForEach` can be used. Select operation will not update value in parent list itself, but return the updated value, that OP has not assigned to any variable, and ultimately we cant find any updated value. – Deepak Sharma Jan 29 '18 at 06:52
  • @DeepakSharma Yes, there is no difference between `ForEach` and `Select` in those pieces of code... and both use `x.IsPriceBook = true` to assign the value... So why again this code would be better than original? – Alexei Levenkov Jan 29 '18 at 06:56
  • i think i can not use .Select() in updating. I had to use ToList() and Foreach() as @Balah mentioned. – user2837480 Jan 29 '18 at 07:03
  • @AlexeiLevenkov you're right - there should have been a bit more explanation. I've updated the answer accordingly. – Balah Jan 29 '18 at 07:16
  • 1
    @user2837480 please ignore explanation provided in this answer and simply use suggested code. There is absolutely no valid statements in the text of this post (code is ok, but `ToList` is wasteful - see my answer). – Alexei Levenkov Jan 29 '18 at 09:07
  • @DeepakSharma I've posted explanation (`Select` actually partly to blame, but not for the reason in your comment). And my comment " no difference between ForEach and Select" is wrong - lazy execution part is important for this case. – Alexei Levenkov Jan 29 '18 at 09:11