Can this LINQ query be simplified?

Question

Coming from Java, I am new to C# and LINQ. There are many queries in our code base which seem to not be optimally constructed. In the following query the GroupBy lambda expression creates an anonymous data type. I can't find any examples online where GroupBy is used like this. Is there a way to simplify this query and still return the same results?

 List<MachineMetrics> machines = prod.Where(p => p.TimeProduced >= start &&
                                      p.TimeProduced <= end &&
                                      (jobID == -1 ? true : (p.JobID == jobID && p.Job.MachineID == MachineID))).
                                GroupBy(x => new
                                             {
                                                 MachineName = x.Job.Machine.MachineName,
                                                 MachineID = x.Job.MachineID,
                                                 JobName = x.Job.JobName,
                                                 JobID = x.JobID
                                              }).
                                Select(item => new MachineMetrics()
                                              {
                                                 MachineName = item.Key.MachineName,
                                                 MachineID = item.Key.MachineID,
                                                 JobName = item.Key.JobName,
                                                 JobID = item.Key.JobID
                                              }).
                                ToList<MachineMetrics>();

edit: Thanks for the help. The problem was the Equals() and GetHashCode() methods were not implimented for the class. Once I added those I used the code suggested by @Ladislav Mrnka and everything worked as expected.

[Here is an example of GroupBy being used like this](http://stackoverflow.com/questions/847066/group-by-multiple-columns-linq) (different syntax, but same outcome) — Jason Kleban, Aug 10 '11 at 12:21
But why assign a value to the variables? The example in the link just shows: group x by new { x.Column1, x.Column2 } — Jason, Aug 10 '11 at 12:26
What's the purpose of the group? seeing that you never actually use the group items — Rune FS, Aug 10 '11 at 12:40
@Jason - To answer your question "why assign a value to the variables?" - That is a good observation. This assignment is actually unnecessary in this case. The compiler will automatically name the fields the same as the property they are being assigned from (x.Job.Whatever, in this example), which are all the same names. If you wanted to change a property name, then it could be done this way (`JobIdentifier = x.JobID`). These names are reused in your `.Select()` where it again reassigns the properties like: `new MachineMetrics { MachineName = item.Key.MachineName` — CodingWithSpike, Aug 10 '11 at 12:46

score 4 · Accepted Answer · answered Aug 10 '11 at 12:36

4

You are looking for this:

List<MachineMetrics> machines = prod.Where(p => p.TimeProduced >= start &&
                                                p.TimeProduced <= end &&
                                                (jobID == -1 || 
                                                    (p.JobID == jobID && p.Job.MachineID == MachineID))).
                                    .Select(x => new MachineMetrics()
                                          {
                                             MachineName = x.Job.Machine.MachineName,
                                             MachineID = x.Job.MachineID,
                                             JobName = x.Job.JobName,
                                             JobID = x.JobID
                                          })
                                    .Distinct()
                                    .ToList();

answered Aug 10 '11 at 12:36

Ladislav Mrnka

360,892
59
660
670

+1. Note that the original query groups by every selected 'column' or property, which is the same as just doing a `.Distinct()`, Also `ToList()` was changed to just `ToList()` because the underlying generic type (MachineMetrics) is already known by the compiler so does not need to be specified (because that is the return type from the preceding `.Select()`. ... Thought I would add this further explanation since the OP said they were new to .NET and Linq. – CodingWithSpike Aug 10 '11 at 12:40
That is what I originally thought, but in the original code machine has a list count of 1, but with your code machine has a list count >1 (it had a list count of 42 last run) – Jason Aug 10 '11 at 12:42
@rally25rs - it seems like the distinct method is not working corretly. the machines list contains identical objects at every index. – Jason Aug 10 '11 at 13:34
1

If the Distinct is not working correctly then it is probably an issue with object comparison. You can try creating a new class that implements [IEqualityComparer](http://msdn.microsoft.com/en-us/library/ms132151.aspx) of type MachineMetrics. This allows you to define what constitutes equality for this object. You'll pass this new class as a parameter to Distinct. – avanek Aug 10 '11 at 16:00

score 2 · Answer 2 · answered Aug 10 '11 at 12:55

Ladislav's answer is good, but just to show another alternative, preserving the GroupBy, you could reduce it to:

var machines = prod.Where(p => p.TimeProduced >= start &&
                               p.TimeProduced <= end &&
                               (jobID == -1 ? true : (p.JobID == jobID && p.Job.MachineID == MachineID))).
                    GroupBy(x => new MachineMetrics
                                 {
                                     MachineName = x.Job.Machine.MachineName,
                                     MachineID = x.Job.MachineID,
                                     JobName = x.Job.JobName,
                                     JobID = x.JobID
                                  }).
                    Select(item => item.Key). // 'item' is the grouping, and its 'Key' is the 'MachineMetrics' instance
                    ToList();

Can this LINQ query be simplified?

2 Answers2