2

I'm doing a simple GroupBy taking the First element but I want to modify one of the property of each result.

class M
{
 public string Name {get; set;}
 public int NOfPeopleWithTheSameName {get; set;}
 public string P1 {get; set;}
 public string P2 {get; set;}
 public string P3 {get; set;}
 public string P4 {get; set;}
 public string P5 {get; set;}
}  


List<M> myList = GetMyList();


var l2 = myList
   .GroupBy(m => m.Name)
   .Select(group => new M { Name = group.Key, NOfPeopleWithTheSameName = group.Count() });

This is pretty easy, but this way is not the best if the class has many properties (since every property value should be copied to the new one)? I should copy them one by one.

I would like to simply take the element and change the property NOfPeopleWithTheSameName

Revious
  • 7,816
  • 31
  • 98
  • 147
  • It depends on whether you want new `M` objects or if you'd be OK with mutating the existing ones. – juharr Dec 15 '17 at 16:13
  • Are you saying that `myList` is a list of type `M`? – DavidG Dec 15 '17 at 16:13
  • Yes, myList is of type M – Revious Dec 15 '17 at 16:16
  • 1
    Another option would be to create a constructor on `M` that takes a `M` object and does the copying for you, then you'd just update the one property. That or look into a mapping library like Automapper. – juharr Dec 15 '17 at 16:19
  • 1
    The problem is that your class stores more than it should. It should not know how much duplicates there are in any other collection that is not part of this class. `NOfPeopleWithTheSameName ` does not belong into this class. Remove it and you also solve your problem because you don't need to create new instances of it. – Tim Schmelter Dec 15 '17 at 16:25
  • @TimSchmelter: yes, you are right, it's just an example (probably I could have chosen better) – Revious Dec 15 '17 at 16:37

2 Answers2

2

Yes, you can...

class Test 
{
    public string Name { get; set; }
    public int Number { get; set; }
}

var tests = new List<Test> { /* ... your data here ... */ };

var modifiedTests = tests.Select(test => { test.Name = "MODIFIED"; return test; });

// this actually executes above query and modifies your original items in the list:
var modifiedMaterialized = modifiedTests.ToList();

But you really, really(!) should not!

LinQ is language integrated query. Having a query with side effects is evil. Mustache twisting evil. Just don't, you will save yourself a lot of pain.


I think what you want is just not LinQ, but regular loops:

class M
{
 public string Name {get; set;}
 public int NOfPeopleWithTheSameName {get; set;}
 public string P1 {get; set;}
 public string P2 {get; set;}
 public string P3 {get; set;}
 public string P4 {get; set;}
 public string P5 {get; set;}
}  

List<M> myList = GetMyList();

var groups = myList.GroupBy(m => m.Name).ToList();

foreach(var group in groups)
{
    foreach(var member in group)
    {
        member.NOfPeopleWithTheSameName = group.Count();
    }
}
nvoigt
  • 75,013
  • 26
  • 93
  • 142
  • Thanks! And can it be combined with the count by the GroupBy? – Revious Dec 15 '17 at 16:19
  • @Revious Maybe it could... but you just shouldn't. updating data is the job of a regular loop. Just write one. – nvoigt Dec 15 '17 at 16:26
  • Dear downvoter, without leaving a comment *what* to improve, it's hard to improve anything. Feel free to leave some constructive comments so you didn't just waste your internet points for nothing. – nvoigt Dec 17 '17 at 19:48
  • @DavidG Yeah, looks like the Grinch is around here somewhere :) – nvoigt Dec 18 '17 at 08:29
2

You don't need to create a new M, you can return the existing one, for example:

var l2 = myList
   .GroupBy(m => m.Name)
   .Select(group => 
   {
        var m = group.First();
        m.NOfPeopleWithTheSameName = group.Count();
        return m;
   });

However, if you are going to add a property to your model just for this, I would suggest instead having a different class that wraps the initial model and the count - don't pollute your models. For example, you could have a generic wrapper class like this:

public class ModelCount<T>
{
    public T Model { get; set; }
    public int Count { get; set; }
}

And now group like this:

var l2 = myList
   .GroupBy(m => m.Name)
   .Select(group => new ModelCount<M> 
   {
        Model = group.First(),
        Count = group.Count()
   });
DavidG
  • 113,891
  • 12
  • 217
  • 223
  • @TimSchmelter: I will edit the question. I've added 5 properties in the example. Initializing a new object to change the value of a single property could have sense if there were just a few properties, but with 7 or more... – Revious Dec 15 '17 at 16:24
  • @Revious Downvotes are fine, I don't care about the rep, but I do like to know what is wrong with my posts. I'll either fix them, delete them is calmly disagree and let the votes stand. It's not a requirement for someone to comment, and I get why most people don't. – DavidG Dec 18 '17 at 16:17
  • @Revious Not at all, downvotes without explanation are perfectly fine without a comment, I would just like one. – DavidG Dec 18 '17 at 16:38
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/161461/discussion-between-revious-and-davidg). – Revious Dec 18 '17 at 16:45