patterncsharpMinor
Null replacement - Is this LINQ readable?
Viewed 0 times
thislinqreadablenullreplacement
Problem
This is the original version of code written by my coworker that replaces every null cells with an empty string.
My instant reaction: Ugh.
Being a shameless fanatic of LINQ, I decided to rewrite the code using LINQ.
However, I'm wondering if this code is less readable than the above version, even though it is definitely compact.
In my eyes, it's definitely readable as I'm comfortable with LINQ but my coworkers have not even heard of LINQ.
Which is a better change? Should I keep my LINQ version or unroll LINQ to two foreach loops?
for (int i = 0; i < dGV_factory.Rows.Count; i++)
{
this.dGV_factory["dGV_factory_groupID", i].Value
= this.dGV_factory["dGV_factory_groupID", i].Value ?? "";
this.dGV_factory["dGV_factory_groupID", i].Value
= this.dGV_factory["dGV_factory_groupID", i].Value ?? "";
this.dGV_factory["dGV_factory_groupID", i].Value
= this.dGV_factory["dGV_factory_groupID", i].Value ?? "";
this.dGV_factory["UE", i].Value
= this.dGV_factory["UE", i].Value ?? "";
...
}My instant reaction: Ugh.
Being a shameless fanatic of LINQ, I decided to rewrite the code using LINQ.
foreach(DataGridViewRow row in dGV_factory.Rows)
{
//Replace every null cells with an empty string
row.Cells.Cast().ToList().ForEach(cell => cell.Value = cell.Value ?? "");
}However, I'm wondering if this code is less readable than the above version, even though it is definitely compact.
In my eyes, it's definitely readable as I'm comfortable with LINQ but my coworkers have not even heard of LINQ.
Which is a better change? Should I keep my LINQ version or unroll LINQ to two foreach loops?
Solution
List.ForEach might have its uses, but I wouldn't use it here. It's not "wrong" per se, but a classic foreach loop - is at least as easy to read as your LINQ expression,
- does not require the explicit
Cast(),
- does not require
ToList().
If you put it on one line, it is actually shorter than your LINQ-based solution.
foreach(DataGridViewRow row in dGV_factory.Rows)
{
//Replace every null cells with an empty string
foreach (DataGridViewCell cell in row.Cells) cell.Value = cell.Value ?? "";
}Although, for readability, I'd prefer:
foreach (DataGridViewRow row in dGV_factory.Rows)
{
foreach (DataGridViewCell cell in row.Cells)
{
cell.Value = cell.Value ?? "";
}
}(Of course, this is still a great improvement over your co-worker's code.)
Code Snippets
foreach(DataGridViewRow row in dGV_factory.Rows)
{
//Replace every null cells with an empty string
foreach (DataGridViewCell cell in row.Cells) cell.Value = cell.Value ?? "";
}foreach (DataGridViewRow row in dGV_factory.Rows)
{
foreach (DataGridViewCell cell in row.Cells)
{
cell.Value = cell.Value ?? "";
}
}Context
StackExchange Code Review Q#31344, answer score: 9
Revisions (0)
No revisions yet.