2

I have a data structure in the form of a tree. Each node represents a cardboard box. The boxes can contain child boxes.

public class CardboardBox
{
   public int Id {get; set;}
   public int ParentBoxId {get; set;}

   // [...]
}

I'm storing those objects in a flat list rather than a tree structure:

public class CardboardBoxList : List<CardboardBox>
{
}

I want to be able to traverse the tree so I made a 'for each' delgate like this:

public class CardboardBox
{
   // [...]

   public delegate void ForEachDelegate(int childId);

   public void ForEachChildBox(CardboardBoxList fullListOfBoxes, ForEachDelegate forEachDelegate)
   {
      List childIdList = fullListOfBoxes.GetChildIdList(this.Id);

      foreach(int childId in childIdList)
      {
         forEachDelegate(childId);
      }
   }
}

This is all good because now I can easily recurse through the box-tree with:

someBox.ForEachChild(fullListOfBoxes, childId =>
{
   CardboardBox currentBox = fullListOfBoxes.GetById(childId);

   // [... do something with it ...]
};

The problem is, I have code that wants to go through the boxes and build some other data structure. And I need to have some variables outside the delegate call like this:

NewDataStructure newDataStructure = new NewDataStructure();

int currentParentNodeId = newDataStructure.AddRootNode();

someBox.ForEachChild(fullListOfBoxes, childId =>
{
   CardboardBox currentBox = fullListOfBoxes.GetById(childId);

   NewDataStructureNode newNode = new NewDataStructureNode();
   newNode.Color = currentBox.Color;

   newDataStructure.AddNodeToParent(newNode, currentParentNodeId);
};

So it needs to create a closure for currentParentNodeId as it builds the new tree structure. But I want to try to avoid using closures if possible, just to keep the code simple.

I realize closures just come with the territory when using delegates. In a way it is kind of the point of using delegates. You want to have an anonymous method that interacts with the code around it, rather than having to define a whole new method like the old days.

I just wonder if there is a better way here. The CardboardBox tree data structure will be used extensively in this code and will sometimes need closures on all types of different variables, so I can't just add a new parameter to the delegate type. I would need to create a new delegate and a new ForEachChild method for each delegate type.

I could try adding a generic parameter to the delegate and just pass in a kind of context parameter, but it still doesn't feel clean. In the end I'm trying to avoid having any closures if possible but I am not sure if I am trying to hard.

Please note this code is all paraphrased and the (probably lame) example of CardboardBox is really a container-type of object in the real code. The NewDataStructure is also a tree in the real code but it does not necessarily have the same structure as the tree it is built from.

I posted this question on StackOverflow yesterday but the question was deemed too vague, understandably. I have tried to re-write it more clearly but I admit it is still fairly unclear. I have also done a fair bit of searching over the last few days and reading up about delegates, anonymous methods and closures which has been helpful. But I still don't feel comfortable just using closures willy-nilly. So I'd appreciate any advice or just opinions on all this.

Disclaimer: I have used c# for many years so if there are any syntax errors I don't need you to point those out. I am paraphrasing the real code off the top of my head.

5
  • 1
    I'm really confused as to why you describe CardboardBoxes as trees when they keep references to their parent instead of their children, and why you need to to extend List. Wouldn't it be much simpler to actually use a tree? I'm also confused why you want to avoid closures; you don't give any rationale for it.
    – Doval
    Feb 5 '15 at 15:18
  • 1
    The ParentId is used just to mirror a standard database design where you can't really do it the other way. Feb 5 '15 at 15:51
  • I don't have a good reason to avoid closures except to avoid the complexity of it. Complexity in the form of different kinds of states that will be captured by the delegate through this whole project, as I will be using the ForEach delegate in many different places in the code. Let me know if that is still unclear. Feb 5 '15 at 15:53
  • Then you're not trying to avoid all closures, just closures with mutable state, correct? Seems to me your problem is that forEachChild doesn't have a return value, so the only way to get anything "out" of a forEachChild call is to close over a mutable variable. What you probably want is a fold, which is a function that iterates over the data structure while transforming each element and combining the result with an accumulator. In this case the accumulator would be the new data structure.
    – Doval
    Feb 5 '15 at 16:06
  • Thanks Doval I'll look into folds they sound interesting for this application. Feb 5 '15 at 16:22
3

Why don't you design the interface of CardBoardBox utilizing an IEnumerable:

public class CardboardBox
{
   // [...]

   public IEnumerable<int> GetChildBoxIDs(CardboardBoxList fullListOfBoxes)
   {
      List childIdList = fullListOfBoxes.GetChildIdList(this.Id);

      foreach(int childId in childIdList)
      {
         yield return childId;
      }
      // remark: this function could be arbitrarily more complex
   }
}

In your example, the calling code then looks like this:

foreach(var childId in someBox.GetChildBoxIDs(fullListOfBoxes))
{
    CardboardBox currentBox = fullListOfBoxes.GetById(childId);

  // [... do something with it ...]
}

So the caller does not need to provide an anonymous function any more.

2
  • Thanks doc I think you have basically solved my issue perfectly. I'm not sure of standard practice here but I will let the question stay for a couple of days before accepting your excellent suggestion. Feb 5 '15 at 15:54
  • @user4531519: thanks for the feedback. Actually, the whole Linq stuff works like that, so yes, I am pretty sure it is standard.
    – Doc Brown
    Feb 5 '15 at 16:16

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.