Deep nesting when looping over an object model coming from a 3rd part

Question

I am stuck with this horrible object model coming back from a 3rd party product. It's six levels of objects deep, and I have to loop through the collection in each level, to get the values I need, in order to produce a small object model that I actually require.

The code ends up looking like this (with variable and type names changed). How can I clean up this mess when I can't modify the structure of the rootObject?

(This is .NET 3.5.)

var levelOneEnumerator = rootObject.GetEnumerator();
while (levelOneEnumerator.MoveNext())
{
    var levelOneItem = levelOneEnumerator.Current as Foo_LevelOneItem;
    if (levelOneItem == null) continue;

    var levelTwoItemsEnumerator = levelOneItem.LevelTwoItems.GetEnumerator();
    while (levelTwoItemsEnumerator.MoveNext())
    {
        var LevelTwoItemsItem = levelTwoItemsEnumerator.Current as Foo_LevelTwoItem;
        if (LevelTwoItemsItem == null) continue;

        var foobars = new List<FooBar>();

        var levelThreeItemsEnumerator = LevelTwoItemsItem.LevelThreeItems.GetEnumerator();
        while (levelThreeItemsEnumerator.MoveNext())
        {
            var levelThreeItem = levelThreeItemsEnumerator.Current as Foo_LevelThreeItem;
            if (levelThreeItem == null) continue;

            var levelFourItemsEnumerator = levelThreeItem.LevelFourItems.GetEnumerator();
            while (levelFourItemsEnumerator.MoveNext())
            {
                var levelFourItem = levelFourItemsEnumerator.Current as Foo_LevelFourItem;
                if (levelFourItem == null) continue;

                var levelFiveItemsEnumerator = levelFourItem.LevelFiveItems.GetEnumerator();
                while (levelFiveItemsEnumerator.MoveNext())
                {
                    var levelFiveItem = levelFiveItemsEnumerator.Current as Foo_LevelFiveItem;
                    if (levelFiveItem == null) continue;

                    var levelSixItemsEnumerator = levelFiveItem.LevelSixItems.GetEnumerator();
                    while (levelSixItemsEnumerator.MoveNext())
                    {
                        var levelSixItem = levelSixItemsEnumerator.Current as Foo_LevelSixItem;
                        if (levelSixItem == null) continue;

                        var levelSixKey = levelSixItem.Key;
                        var foobar = foobars.Where(x => x.Key == levelSixKey).FirstOrDefault();
                        if (foobar == null)
                        {
                            foobar = new FooBar
                                          {
                                              LevelSixKey = levelSixKey,
                                              TransDate = levelFiveItem.TransDate,
                                              PaidAmount = 0
                                          };
                            foobars.Add(foobar);
                        }

                        // * -1 because value should be positive, while product reports a negative (and vice versa)
                        foobar.PaidAmount += (levelFiveItem.PaidAmount ?? 0) * -1; 
                    }
                }
            }
        }

        yield return new FooBarsCollection
                         {
                             Prop1 = levelTwoItemsItem.Prop1,
                             Prop2 = levelTwoItemsItem.Prop2,
                             Prop3 = levelTwoItemsItem.Prop3,
                             FooBars = foobars
                         };
    }
}

Answer 1

First of all, I would realise that

var levelOneEnumerator = rootObject.GetEnumerator();
while (levelOneEnumerator.MoveNext())
{
    var levelOneItem = levelOneEnumerator.Current as Foo_LevelOneItem;
    if (levelOneItem == null) continue;

is equivalent to the much shorter

foreach (var levelOneItem in rootObject.OfType<Foo_LevelOneItem>())
{

Then you will realise that you have several nested foreach loops which make for nicely-readable single-line nestings. So factor all the multi-line code into methods of its own. The end-result I got looks like this:

foreach (var levelOneItem in rootObject.OfType<Foo_LevelOneItem>())
    foreach (var levelTwoItem in levelOneItem.LevelTwoItems.OfType<Foo_LevelTwoItem>())
        yield return new FooBarsCollection
        {
            Prop1 = levelTwoItem.Prop1,
            Prop2 = levelTwoItem.Prop2,
            Prop3 = levelTwoItem.Prop3,
            FooBars = getFoobars(levelTwoItem)
        };

[...]

private static List<FooBar> getFoobars(Foo_LevelTwoItem levelTwoItem)
{
    var foobars = new List<FooBar>();

    foreach (var levelThreeItem in levelTwoItem.LevelThreeItems.OfType<Foo_LevelThreeItem>())
        foreach (var levelFourItem in levelThreeItem.LevelFourItems.OfType<Foo_LevelFourItem>())
            foreach (var levelFiveItem in levelFourItem.LevelFiveItems.OfType<Foo_LevelFiveItem>())
                foreach (var levelSixItem in levelFiveItem.LevelSixItems.OfType<Foo_LevelSixItem>())
                    processLevelSixItem(foobars, levelFiveItem, levelSixItem.Key);

    return foobars;
}

private static void processLevelSixItem(List<FooBar> foobars, Foo_LevelFiveItem levelFiveItem, Foo_LevelSixItemKey levelSixKey)
{
    var foobar = foobars.Where(x => x.Key == levelSixKey).FirstOrDefault();
    if (foobar == null)
    {
        foobar = new FooBar
        {
            LevelSixKey = levelSixKey,
            TransDate = levelFiveItem.TransDate,
            PaidAmount = 0
        };
        foobars.Add(foobar);
    }

    // * -1 because value should be positive, while product reports a negative (and vice versa)
    foobar.PaidAmount += (levelFiveItem.PaidAmount ?? 0) * -1;
}

Of course, you could further change this into a much more LINQy expression involving either SelectMany or the from query syntax, but to be honest, in your particular case I would leave it like this. It is very clear. In case you still want the query syntax, here is just getFoobars to give you the idea:

private static List<FooBar> getFoobars(Foo_LevelTwoItem levelTwoItem)
{
    var foobars = new List<FooBar>();

    var query =
        from levelThreeItem in levelTwoItem.LevelThreeItems.OfType<Foo_LevelThreeItem>()
        from levelFourItem in levelThreeItem.LevelFourItems.OfType<Foo_LevelFourItem>()
        from levelFiveItem in levelFourItem.LevelFiveItems.OfType<Foo_LevelFiveItem>()
        from levelSixItem in levelFiveItem.LevelSixItems.OfType<Foo_LevelSixItem>()
        select new { LevelFiveItem = levelFiveItem, Key = levelSixItem.Key };

    foreach (var info in query)
        processLevelSixItem(foobars, info.LevelFiveItem, info.Key);

    return foobars;
}

Answer 2

I would probably refactor out the two innermost loops into a method returning a FooBarCollection. You could then collapse the other loops with LINQs SelectMany:

foreach (var level2 in rootObject.SelectMany(l1 => l1.LevelTwoItems)) {
    var items = level2.LevelThreeItems
        .SelectMany(l3 => l3.LevelFourItems)
        .SelectMany(l4 => l4.LevelFiveItems);
    yield return CollectFooBars(level2, items);
}

Edit: I took out the filtering for null values because that seem to be a result of non-generic enumerators in your example (and I assume there aren't any null values in those arrays).

Answer 3

What we have here is a tree data structure. Unfortunately it is an implicit tree structure which means we can't easily write some generic code to search the tree.

Make the tree structure explicit

So, what we first need is an adapter that can help to make things more generic. I don't know if these classes share a common base class, so everything will have to use object. If there is a common base class this will make things much tidier..

We can have a class like so :

public static class BranchExtensions
{
    public static Dictionary<Type, Func<object, IEnumerable>> NextLevels = new Dictionary<Type, Func<object, IEnumerable>> ();

    private static IEnumerable NextLevel ( object branch )
    {
        Func<object, IEnumerable> nextLevel;
        if (NextLevels.TryGetValue(branch.GetType (), out nextLevel))
            return nextLevel ( branch );
        else
            return null;
    }
}

Then we need some initialization code somewhere :

BranchExtensions.NextLevels.Add ( typeof ( Foo_LevelOneItem ), level => ( (Foo_LevelOneItem) level ).Level2Items );
BranchExtensions.NextLevels.Add ( typeof ( Foo_LevelTwoItem ), level => ( (Foo_LevelTwoItem) level ).Level3Items );

Now, given any item in the heirarchy I can call NextLevel and get an Enumerator for its next level.

Write some general utility functions

This enables us to write some generic code.

What we need to do is walk the tree, collecting the objects we want along the way. When we read a leaf node, we want to process.

So we add the following to our BranchExtensions :

    private static IEnumerable<Dictionary<Type, object>> Flatten ( object branch, Predicate<object> collect, Dictionary<Type, object> collection )
    {
        IEnumerable nextLevel = NextLevel ( branch );
        if ( nextLevel != null )
        {
            foreach ( object next in nextLevel )
            {
                if ( next != null )
                {
                    // Do we want to collect this type
                    if ( collect ( next.GetType () ) )
                        collection[next.GetType ()] = next;

                    if ( NextLevel ( next ) == null )
                    {
                        // This is a leaf node.
                        yield return collection;
                        collection.Remove (next.GetType ());
                    }
                    else
                    {
                        // This is a branch, so recurse down the tree.
                        foreach ( var more in Flatten ( next, collect, collection ) )
                        {
                            if ( more != null )
                            {
                                yield return more;
                                collection.Remove ( more.GetType () );
                            }
                        }
                    }
                }
            }
        }
    }

    public static IEnumerable<Dictionary<Type, object>> Flatten ( this Predicate<object> collect, object branch )
    {
        return Flatten ( branch, collect, new Dictionary<Type, object> () );
    }

    public static Predicate<object> Collect ( Predicate<object> collect )
    {
        return collect;
    }

The first Flatten method does all the work. It is pretty horrendous and I'm sure it could be tidied up and refactored, I just hacked it together to demonstrate the point.

The second Flatten method is the entry point method. This creates a new Dictionary to collect our values and passes the call along. It should do some validation here as well really.

As a slight frill - this is an extension method on Predicate. The Collect method following is even stranger and does nothing really. It is just there to enable a more fluent interface when coming to use it.

Use the code

The Nested loops can then be converted to something like the following :

foreach ( var collection in BranchExtensions.Collect( type => type == typeof(Foo_LevelFiveItem) || type == typeof(Foo_LevelSixItem)).Flatten ( rootObject ) )
{
    var levelFiveItem = collection[typeof(Foo_LevelFiveItem)] as Foo_LevelFiveItem;
    var levelSixItem = collection[typeof(Foo_LevelSixItem)] as Foo_LevelSixItem

    // Do stuff on our items...
}

It looks like a lot of work to acheive a small thing. It all depends on how much you need to use this stuff. If you have only got a single nested loop in your entire application, then none of this makes sense and I would just suck up the nested loop and carry on.

But,

If you have nested loops all over your code, then a few utility methods like this would make a lot of sense to abstract the complexity away and allow you to focus on the meaning of what you are trying to do.

Answer 4

If you extract the innermost nested part as its own method that takes a LevelSixKey as its parameter, then create your own Enumerator over levelSixItems, the product of which is the same as all your nesting (and, honestly, simply moved all the nesting into it - or added to a list at the inside of the innermost loop, and returned that list's enumerator), then what you see for the foobar-building is simply iterating over the enumerator and doing what you do - and all the ugly nested stuff is hidden away out of sight. It doesn't exactly solve the problem, but maybe it makes it more manageable.

Answer 5

I've not tested it properly, but this LINQ query should do the same:

foreach (var levelOneItem in rootObject.OfType<Foo_LevelOneItem>()) {
    foreach (var levelTwoItemsItem in levelOneItem.LevelTwoItems.OfType<Foo_LevelTwoItem>()) {

        var foobars = (from levelThreeItem in levelTwoItemsItem.LevelThreeItems.OfType<Foo_LevelThreeItem>()
                       from levelFourItem in levelThreeItem.LevelFourItems.OfType<Foo_LevelFourItem>()
                       from levelFiveItem in levelFourItem.LevelFiveItems.OfType<Foo_LevelFiveItem>()
                       from levelSixItem in levelFiveItem.LevelSixItems.OfType<Foo_LevelSixItem>()
                       group levelSixItem by levelSixItem.levelSixKey into groupedLevelSixItems
                       select new FooBar() {
                           LevelSixKey = groupedLevelSixItems.Key,
                           TransDate = levelFiveItem.TransDate,
                           PaidAmount = (-levelFiveItem.PaidAmount ?? 0) * groupedLevelSixItems.Count()
                       }).ToList();

        yield return new FooBarsCollection
                         {
                             Prop1 = levelTwoItemsItem.Prop1,
                             Prop2 = levelTwoItemsItem.Prop2,
                             Prop3 = levelTwoItemsItem.Prop3,
                             FooBars = foobars
                         };
    }
}