When looping over a collection in a foreach()
loop, there could be an issue with casting that coders might not be aware of. Let’s look at the code that demonstrates this issue.
foreach (DataInstance item in instances.DataItemInstances)
{
if (item.Definition != null && item.Definition.External)
{
results.Add(item);
}
}
Upon examining this code, it’s not immediately apparent that DataItemInstances
is a collection type implementing ICollection<>
of DataInstance
. The issue lies in the fact that it obscures a potentially problematic cast, which may lead to runtime issues. I assume that most developers reviewing this code might not readily recognize this issue; I had to delve deeper to discern it.
To rectify this code, you can simply add an explicit cast, as demonstrated in the refactored code below:
foreach (var item in instances.DataItemInstances.Cast<DataInstance>())
{
if (item.Definition is not null && item.Definition.External)
{
results.Add(item);
}
}
This code can be further simplified by employing LINQ, as demonstrated below:
results.AddRange(instances.DataItemInstances.Cast<DataInstance>()
.Where(item => item.Definition is not null &&
item.Definition.External));
Utilizing AddRange()
can significantly enhance the efficiency of this code!
This is related to violation IDE0220 and this is how I have it setup in my .editorConfig: dotnet_diagnostic.IDE0220.severity = warning
Summary
While reviewing the codebase I utilized for this article, I discovered 98 occurrences where the code needs to be fixed.
For further guidance and insights, I highly recommend obtaining a copy of my book, “Rock Your Code: Coding Standards for Microsoft .NET” available on Amazon.com. Additionally, to explore more performance tips for .NET, I encourage you to acquire the 3rd edition of “Rock Your Code: Code & App Performance for Microsoft .NET” also available on Amazon.com.
To analyze your code using the same settings I used in these articles, I encourage you to incorporate my EditorConfig file. It can be found at the following link: https://bit.ly/dotNetDaveEditorConfig. I update this file quarterly, so remember to keep yours up to date as well. I hope you will check out my OSS project Spargine by using this link: https://bit.ly/Spargine.
Please feel free to leave a comment below. I would appreciate hearing your thoughts and feedback.
Pick up any books by David McCarter by going to Amazon.com: http://bit.ly/RockYourCodeBooks
Make a one-time donation
Make a monthly donation
Make a yearly donation
Choose an amount
Or enter a custom amount
Your contribution is appreciated.
Your contribution is appreciated.
Your contribution is appreciated.
DonateDonate monthlyDonate yearlyIf you liked this article, please buy David a cup of Coffee by going here: https://www.buymeacoffee.com/dotnetdave
© The information in this article is copywritten and cannot be preproduced in any way without express permission from David McCarter.
Hi, I would argue that someone behind the scenes changing the type of the collection would be quite a rare incident/accident.
I understand that in some environments the explicit cast will provide a good level of extra security, but I just really like the simplicity of:
foreach (var item in instances.DataItemInstances)
which I think would benefit most.
I might be too naive. Maybe you have a good story to set me straight? 🙂