-5

Please do not get intimidated by the wall of code! My question simply put would be the following - can I somehow reuse the code and place whatever arguments I want in the place of // DoSomething? I do not want to copy and paste the same wall of code, I want to make it less repetitive and easier to maintain. I have wondered about this problem, but I do not see a straightforward solution and I am unable to find one.

for (int i = 0; i < treeListViewDatabase.SelectedObjects.Count; i++)
            {
                if (treeListViewDatabase.SelectedObjects[i] is NodeValueWithNodes rootNode)
                {
                    for (int m = 0; m < rootNode.ChildTreeViewSets.Count; m++)
                    {
                        if (rootNode.ChildTreeViewSets[m] is NodeValueWithNodes childNodeWithNode)
                        {
                            for (int k = 0; k < childNodeWithNode.ChildTreeViewSets.Count; k++)
                            {
                                if (childNodeWithNode.ChildTreeViewSets[k] is NodeValueWithNodes secondChildNodeWithNode)
                                {
                                    for (int p = 0; p < secondChildNodeWithNode.ChildTreeViewSets.Count; p++)
                                    {
                                        if (secondChildNodeWithNode.ChildTreeViewSets[p] is NodeValueWithDevices thirdChildNodeWithDevices)
                                        {
                                            for (int r = 0; r < thirdChildNodeWithDevices.ChildDeviceSet.Count; r++)
                                            {
                                                if (dataRows.Contains(thirdChildNodeWithDevices.ChildDeviceSet[r]))
                                                {
                                                    dataRows.Remove(thirdChildNodeWithDevices.ChildDeviceSet[r]);
                                                }
                                                // DoSomething() 
                                            }
                                        }
                                    }
                                }
                                if (childNodeWithNode.ChildTreeViewSets[k] is NodeValueWithDevices secondChildNodeWithDevice)
                                {
                                    for (int r = 0; r < secondChildNodeWithDevice.ChildDeviceSet.Count; r++)
                                    {
                                        if (dataRows.Contains(secondChildNodeWithDevice.ChildDeviceSet[r]))
                                        {
                                            dataRows.Remove(secondChildNodeWithDevice.ChildDeviceSet[r]);
                                        }
                                        // DoSomething();
                                    }
                                }
                            }
                        }
                        if (rootNode.ChildTreeViewSets[m] is NodeValueWithDevices childNodeDevices)
                        {
                            for (int n = 0; n < childNodeDevices.ChildDeviceSet.Count; n++)
                            {
                                if (dataRows.Contains(childNodeDevices.ChildDeviceSet[n]))
                                {
                                    dataRows.Remove(childNodeDevices.ChildDeviceSet[n]);
                                }
                                // DoSomething();
                            }
                        }
                    }
                }
                if (treeListViewDatabase.SelectedObjects[i] is NodeValueWithDevices rootNodeWithDevices)
                {
                    for (int n = 0; n < rootNodeWithDevices.ChildDeviceSet.Count; n++)
                    {
                        if (dataRows.Contains(rootNodeWithDevices.ChildDeviceSet[n]))
                        {
                            dataRows.Remove(rootNodeWithDevices.ChildDeviceSet[n]);
                        }
                        // DoSomething();
                    }
                }
            }
            for (int i = 0; i < dataRows.Count; i++)
            {
                // DoSomething();
            }

*EDIT*

Following the advise of the people I have completely changed the code. The arrow-like look to the code was replaced with a simple recursion. This implementation made my issue non-existent as I can loop through the dataRows to perform the necessary action.

private void TraverseTreeView(IList selectedObjects, List<DataRow> dataRows)
    {
        if (selectedObjects == null)
        {
            return;
        }
        foreach (var selectedObject in selectedObjects)
        {
            if (selectedObject is NodeValueWithNodes withNodes)
            {
                TraverseTreeView(withNodes.ChildTreeViewSets, dataRows);
            }
            if (selectedObject is NodeValueWithDevices withDevices)
            {
                TraverseTreeView(withDevices.ChildDeviceSet, dataRows);
            }
            if (selectedObject is DataRow dataRow && !dataRows.Contains(dataRow))
            {
                dataRows.Add(dataRow);
            }
        }
        
    }
  • 3
    _" I want to make it less repetitive and easier to maintain"_ I would start by trying to get rid of the deeply nested loops and `if` blocks. You can achieve that by refactoring several parts of that code into separate methods. Regarding your original question, yes, you can do that by moving the code into a method that accepts an `Action` or `Func` parameter. See [this related post](https://stackoverflow.com/q/2082615/8967612). – 41686d6564 stands w. Palestine Nov 16 '20 at 20:07
  • 1
    Seems you want a _recursive_ function that operates on an instance of `NodeValueWithNodes`. Consider refactoring so that you check first if your node is a `NodeValueWithDevices`, and if so, you're done and don't have to recurse further. If not, then test if that instance is `NodeValueWithNodes`, and if so, call the _same recursive function_ again to check if the nodes inside your node are `NodeValueWithDevices`, and so on. (You'd still need a delegate to inject your changeable `DoSomething()` as per @JoelCoehoorn's answer.) – Sean Skelly Nov 16 '20 at 20:18

2 Answers2

3

You can use a delegate. It might look something like this:

public void TraverseTree(TreeListView root, Action<T> toDo)
{
   //big set of loops goes in here

   //DoSomething(); is replaced with:
   toDo(rootNodeWithDevices.ChildDeviceSet[n]);
}

And then you can call it with a lambda expression, like this:

Traverse(treeListViewDatabase, (node) => {
    //code for whatever you want to do goes in here
});

I would also tend to first re-write the initial set of loops like this:

var items = treeListViewDatabase.SelectedObjects.
    Where(o => o is NodeValueWithNodes).
    Select(o => o.ChildTreeViewSets).
    Where(o => o is NodeValueWithNodes).
    Select(o => o.ChildTreeViewSets).
    Where(o => o is NodeValueWithNodes).
    Select(o => o.ChildTreeViewSets).
    Where(o => o is NodeValueWithDevices).
    Select(o => o.ChildDeviceSet);

foreach(var item in items)
{
    if (dataRows.Contains(item))
        dataRows.Remove(item);
    //DoSomething();
}

If I absolutely had to I'd add a ToList() call to avoid modifying this sequence while iterating.

Having done this much I would further look to build a recursive method to avoid even that much repetition. For example, I'm not sure exactly which third-party TreeListView control you have, but if it were a standard TreeView control I might do this:

public IEnumerable<TreeNode> Traverse(TreeView source)
{
    // See Local Functions:
    // https://learn.microsoft.com/en-us/dotnet/csharp/programming-guide/classes-and-structs/local-functions
    IEnumerable<TreeNode> innerTraverse(TreeNodeCollection root)
    {
        foreach(var node in root)
        {
            yield return node;
            foreach(var node in innerTraverse(root))
            {
                yield return node;
            }
        }
    }

    return innerTraverse(source.nodes));
}

and then use it like this:

var items = Traverse(treeListViewDatabase).
    Where(o => o is NodeValueWithDevices).
    Select(o => o.ChildDeviceSet);
foreach(var item in items)
{
    if (dataRows.Contains(item))
        dataRows.Remove(item);
    //DoSomething();
}

Which isn't exactly equivalent — we haven't yet accounted for selected items and their children (which I would do by building a bool IsParentNodeSelected() method) — but it's definitely starting to get us to more functional and re-usable code.

Joel Coehoorn
  • 399,467
  • 113
  • 570
  • 794
0

Consider using a recursive function (a function that calls itself). For example, to iterate through a Treeview and return a list of ALL of the nodes and their children, and their children's children, etc...:

private List<TreeNode> GetAllNodes(TreeNodeCollection nodes)
{
    List<TreeNode> retNodes = new List<TreeNode>();
    foreach (TreeNode node in nodes)
    {
        retNodes.Add(node);
        retNodes.AddRange(GetAllNodes(node.Nodes));
    }
    return retNodes;
}
JesseChunn
  • 555
  • 3
  • 7