simplify sln-remove implementation#45797
Conversation
Forgind
left a comment
There was a problem hiding this comment.
I don't really have a strong preference between these two versions, to be honest. I think edvilme's version is a bit more understandable, especially as I normally think tree structures lend themselves well to recursive procedures. kasperk81's version should be faster and allocate less.
using .Where.GroupBy.ToDictionary then keeping track of caches in other dictionaries with a recursive call is significantly worse than this simple loop in every regard. this version offers implementation that has less lines of codes, less calls, performant and reduced allocations |
| nonFolderDescendants += current.Files?.Count ?? 0; | ||
| foreach (var child in solution.SolutionItems) | ||
| { | ||
| if (child is { Parent: var parent } && parent == current) |
|
This looks good to me, my main concern would most likely be readability, but I understand how many of the operations can come at a cost, especially for really large solution files |
|
i'm not sure i'm following. this version is more readable with a single loop compared to multiple linq expressions, caching and recursion |
complex linq expressions, multiple dictionaries and recursion is hardly necessary for this simple command