False if-statement still executes

Question

I have the following method which should iterate through a tree while invoking actions on each node plus once every time it steps up or down the hierarchy. The logic itself works flawless, but I realized that my unit test fails because the if-statement which checks for already invoked elements fails to do what it is logical meant to do.

public static void IterateWithHierarchyFeedback(TBase start,
    Func<TBase, IEnumerable<TBase>> childSelector,
    Action<TBase> invoker,
    Action stepUpInvocator,
    Action stepDownInvocator,
    bool invokeOnStart)
{
    Stack<DynamicIterator> iterationStack = new Stack<DynamicIterator>();
    iterationStack.Push(new DynamicIterator(start, childSelector(start).GetEnumerator()));

    while (iterationStack.Count > 0)
    {
        var current = iterationStack.Pop();
        // HERE it fails because current.Invoked = true but invoker() is still executed
        if (!current.Invoked && invokeOnStart || current.Current != start)
            invoker(current.Current);

        if (current.Enumerator == null || !current.Enumerator.MoveNext())
        {
            if (current.Current != start)
                stepUpInvocator();
            continue;
        }

        stepDownInvocator();

        current.Invoked = true;
        iterationStack.Push(current);
        iterationStack.Push(new DynamicIterator(current.Enumerator.Current,
            childSelector(current.Enumerator.Current)?.GetEnumerator()));
        continue;
    }
}

This is my unit test:

[Test] public async Task TestIterateWithFeedback() { StringBuilder b = new StringBuilder();

DynamicIterate<Tree>.Downwards.IterateWithHierarchyFeedback(_tree, t => t.Children,
    tree => b.Append(tree.ReturnValue.ToString()),
    () => b.Append('<'),
    () => b.Append('>'),
    true);

Assert.Warn(b.ToString());
const string expected = "1>2>3>4<>5<<>6<<>7>>";
Assert.AreEqual(expected, b.ToString());

}

And here you see that the output is not what it should be. That's because invoker() is called on elements that were already invoked, otherwise the output would be correct when it comes to order:

  2)   Expected string length 20 but was 23. Strings differ at index 8.
  Expected: "1>2>3>4<>5<<>6<<>7>>"
  But was:  "1>2>3>4<3>5<3<2>6<2<>7<"

Can anyone explain to me why this happens? I found this but even without debugger it occurs. I also tried to change my state object (DynamicIterator) from struct to class (as I initially thought this might be an issue with the async variation. But that's not changing anything either.

Answer 1

I'm not sure that the problem is, but the iteration code looks more complicated than needed. I would propose something like this:

    public static IEnumerable<(T Node, int Level)> DepthFirstWithlevel<T>(
           T self, 
           Func<T, IEnumerable<T>> selector)
    {
        var stack = new Stack<(T Node, int Level)>();
        stack.Push((self, 0));
        while (stack.Count > 0)
        {
            var current = stack.Pop();
            yield return current;
            foreach (var child in selector(current.Node))
            {
                stack.Push((child, current.Level + 1));
            }
        }
    }

This would return each node in the tree, together with the level in the tree the node has, with 0 being the root. If you specifically need methods to be called for each level change, you can make that with a separate method, something like this:

    public static void IterateWithHierarchyFeedback<T>(
        T self, 
        Func<T, IEnumerable<T>> selector, 
        Action<T> invoker, 
        Action stepUp, 
        Action stepDown)
    {
        int currentLevel = 0;
        foreach (var (node, level) in DepthFirstWithLevel(self, selector))
        {
            while (currentLevel < level)
            {
                currentLevel++;
                stepDown();
            }

            while (currentLevel > level)
            {
                currentLevel--;
                stepUp();
            }

            invoker(node);
        }
    }

For the tree

A - B - C
  ｜  ⌞ D
   ⌞E - F

It will print A>E>F<B>DC, i.e. It will traverse the bottom branches first (insert a .Reverse() after selector(current.Node) to change this). And it will go from the F to B directly, without revisiting A.