Output Delayed Iterative Issues

Question

I know return returns use lazy loading, but I wonder if I can use the iterator incorrectly or maybe refactoring is needed.

My recursive iterator method returns all the ancestors of the given PageNode , including the PageNode itself.

 public class PageNodeIterator { //properties and constructor left out for brevity public IEnumerable<IPageNode> ancestorsOf(IPageNode pageNode) { if(pageNode == null) throw new ArgumentNullException(("pageNode")); if (pageNode.url != pageNodeService.rootUrl) { yield return pageNode; if (pageNode.parent != null) foreach (var node in ancestorsOf(pageNode.parent)) yield return node; } } } 

In my ancestorsOf call, I call the method and then change the order of the returned IEnumerable , but since loading is delayed, the call does not actually happen until I call ToArray() on the next line and at this point the pageNodeService is set to null in my iterator method and throw an exception zero reference.

 ancestors = pageNodeIterator.ancestorsOf(currentNode).Reverse(); return ancestors.ToArray()[1].parent.children; 

So, I wonder where I made a mistake. What would be the right way to use an iterator in this case, if at all?

I am also wondering why pageNodeService is null at runtime. Even execution is delayed, shouldn't it still hold value?

Answer 1

Not quite the answer ... more suggestions for an alternative implementation that eliminates recursion. Too long to post as a comment.

  public IEnumerable<IPageNode> ancestorsOf(IPageNode pageNode) { if(pageNode == null) throw new ArgumentNullException(("pageNode")); Stack<IPageNode> stack = new Stack<IPageNode>(); stack.Push(pageNode); while(stack.Any()) { IPageNode n=stack.Pop(); if (n.url != pageNodeService.rootUrl) { yield return n; if(n.parent != null) { stack.Push(n.parent); } } } } 

Thinking about this, you can completely remove Stack:

 public IEnumerable<IPageNode> ancestorsOf(IPageNode pageNode) { if(pageNode == null) throw new ArgumentNullException(("pageNode")); IPageNode n = pageNode; while(n != null && n.url != pageNodeService.rootUrl) { yield return n; n = n.parent; } } 

Answer 2

I do not know where your error is, and StackOverflow is not a service for debugging your code; I would solve your problem by running it in the debugger and looking for an error.

However, I will take the opportunity to indicate that this:

 public IEnumerable<IPageNode> AncestorsOf(IPageNode pageNode) { if(pageNode == null) throw new ArgumentNullException(("pageNode")); // Do stuff that yields 

a bit problematic, because none of the code in the block runs until MoveNext is called for the first time. In other words, if you do this:

 var seq = AncestorsOf(null); // Not thrown here! using (var enumtor = seq.GetEnumerator()) { bool more = enumtor.MoveNext(); // Exception is thrown here! 

which is very surprising for people. Instead, write your code as follows:

 public IEnumerable<IPageNode> AncestorsOf(IPageNode pageNode) { if(pageNode == null) throw new ArgumentNullException(("pageNode")); return AncestorsOfIterator(pageNode); } private IEnumerable<IPageNode> AncestorsOfIterator(IPageNode pageNode) { Debug.Assert(pageNode != null); // Do stuff that yields } 

Answer 3

Does it make sense to use the income in this place - since when you call Reverse, all things should be buffered anyway , so that instead you can simply return the full list of ancestors.

Answer 4

Add a start node outside of this iterator if you need it.

 public class PageNodeIterator { //properties and constructor left out for brevity public IEnumerable<IPageNode> ancestorsOf(IPageNode pageNode) { if(pageNode == null) throw new ArgumentNullException(("pageNode")); if (pageNode.url != pageNodeService.rootUrl) { if (pageNode.parent != null ) { yield return pageNode.parent; yield return ancestorsOf(pageNode.parent); } } } }