patterncsharpMinor
To LINQ or not to LINQ
Viewed 0 times
linqnotstackoverflow
Problem
As a fairly new C# programmer I am a unsure about when and how to best make use of LINQ (and also when to choose the expression method syntax vs. the query syntax). This question often comes up for me at work, because I heavily lean toward making full use of new language features and a functional style, while my colleagues seem to prefer a more conservative style. This means I have to think a lot about when using LINQ is actually worth it.
Now here is where the review part comes in. This is an anonymized piece of code that this question came up for, once coded in a LINQ style and once in imperative style. Which version do you believe is preferable from a readability / maintainability viewpoint?
Now here is where the review part comes in. This is an anonymized piece of code that this question came up for, once coded in a LINQ style and once in imperative style. Which version do you believe is preferable from a readability / maintainability viewpoint?
void updateItems()
{
Items.Clear();
Items.AddRange(Widget.GetDetectedWidgets()
.Intersect(GetRelevantWidgets())
.Select(widget => readSerialNr(widget))
.Where(serial => serial != null)
.DefaultIfEmpty("No relevant widgets found.")
.ToArray());
}
void updateItems()
{
Items.Clear();
List relevantWidgets = GetRelevantWidgets().ToList();
foreach(string widget in Widget.GetDetectedWidgets())
{
if(relevantWidgets.Contains(widget))
{
string serial = readSerialNr(widget);
if(serial != null)
Items.Add(serial);
}
}
if(Items.Count == 0)
Items.Add("No relevant widgets found.");
}Solution
Some notes:
-
I think the LINQ version would be more readable if you extracted a variable for the collection. Something like:
-
The LINQ version might be more efficient if there were lots of widgets, because
With the mentioned changes, I think the LINQ version will be more readable, primarily because
Also don't forget that you don't have to use only LINQ or only imperative code, you can combine the two in a single method (e.g. improve the imperative approach by using
-
I think the LINQ version would be more readable if you extracted a variable for the collection. Something like:
var serials = /* the whole LINQ query here */
Items.Clear();
Items.AddRange(serials);-
The LINQ version might be more efficient if there were lots of widgets, because
Intersect() uses hashing to achieve O(n+m) time complexity, while your imperative code is O(n·m).- You don't need
ToArray()in your LINQ version. (AssumingItemsisList.)
- I really don't think the string
"No relevant widgets found."should be the default value if the list is empty. List of widget serial numbers should contain widget serial numbers, nothing else. And you should separate your business logic and presentation.
With the mentioned changes, I think the LINQ version will be more readable, primarily because
Intersect() explains what you want to do better.Also don't forget that you don't have to use only LINQ or only imperative code, you can combine the two in a single method (e.g. improve the imperative approach by using
Intersect()).Code Snippets
var serials = /* the whole LINQ query here */
Items.Clear();
Items.AddRange(serials);Context
StackExchange Code Review Q#21359, answer score: 7
Revisions (0)
No revisions yet.