How can I avoid this infinite loop?

Question

It feels like there must be some semi-simple solution to this, but I just can't figure it out.

Edit: The previous example showed the infinite loop more clearly, but this gives a bit more context. Check out the pre-edit for a quick overview of the problem.

The following 2 classes represent the View-Models of the Model View View-Model (MVVM) pattern.

/// <summary>
/// A UI-friendly wrapper for a Recipe
/// </summary>
public class RecipeViewModel : ViewModelBase
{
    /// <summary>
    /// Gets the wrapped Recipe
    /// </summary>
    public Recipe RecipeModel { get; private set; }

    private ObservableCollection<CategoryViewModel> categories = new ObservableCollection<CategoryViewModel>();

    /// <summary>
    /// Creates a new UI-friendly wrapper for a Recipe
    /// </summary>
    /// <param name="recipe">The Recipe to be wrapped</param>
    public RecipeViewModel(Recipe recipe)
    {
        this.RecipeModel = recipe;
        ((INotifyCollectionChanged)RecipeModel.Categories).CollectionChanged += BaseRecipeCategoriesCollectionChanged;

        foreach (var cat in RecipeModel.Categories)
        {
            var catVM = new CategoryViewModel(cat); //Causes infinite loop
            categories.AddIfNewAndNotNull(catVM);
        }
    }

    void BaseRecipeCategoriesCollectionChanged(object sender, NotifyCollectionChangedEventArgs e)
    {
        switch (e.Action)
        {
            case NotifyCollectionChangedAction.Add:
                categories.Add(new CategoryViewModel(e.NewItems[0] as Category));
                break;
            case NotifyCollectionChangedAction.Remove:
                categories.Remove(new CategoryViewModel(e.OldItems[0] as Category));
                break;
            default:
                throw new NotImplementedException();
        }
    }

    //Some Properties and other non-related things

    public ReadOnlyObservableCollection<CategoryViewModel> Categories 
    {
        get { return new ReadOnlyObservableCollection<CategoryViewModel>(categories); }
    }

    public void AddCategory(CategoryViewModel category)
    {
        RecipeModel.AddCategory(category.CategoryModel);
    }

    public void RemoveCategory(CategoryViewModel category)
    {
        RecipeModel.RemoveCategory(category.CategoryModel);
    }

    public override bool Equals(object obj)
    {
        var comparedRecipe = obj as RecipeViewModel;
        if (comparedRecipe == null)
        { return false; }
        return RecipeModel == comparedRecipe.RecipeModel;
    }

    public override int GetHashCode()
    {
        return RecipeModel.GetHashCode();
    }
}

.

/// <summary>
/// A UI-friendly wrapper for a Category
/// </summary>
public class CategoryViewModel : ViewModelBase
{
    /// <summary>
    /// Gets the wrapped Category
    /// </summary>
    public Category CategoryModel { get; private set; }

    private CategoryViewModel parent;
    private ObservableCollection<RecipeViewModel> recipes = new ObservableCollection<RecipeViewModel>();

    /// <summary>
    /// Creates a new UI-friendly wrapper for a Category
    /// </summary>
    /// <param name="category"></param>
    public CategoryViewModel(Category category)
    {
        this.CategoryModel = category;
        (category.DirectRecipes as INotifyCollectionChanged).CollectionChanged += baseCategoryDirectRecipesCollectionChanged;

        foreach (var item in category.DirectRecipes)
        {
            var recipeVM = new RecipeViewModel(item); //Causes infinite loop
            recipes.AddIfNewAndNotNull(recipeVM);
        }
    }

    /// <summary>
    /// Adds a recipe to this category
    /// </summary>
    /// <param name="recipe"></param>
    public void AddRecipe(RecipeViewModel recipe)
    {
        CategoryModel.AddRecipe(recipe.RecipeModel);
    }

    /// <summary>
    /// Removes a recipe from this category
    /// </summary>
    /// <param name="recipe"></param>
    public void RemoveRecipe(RecipeViewModel recipe)
    {
        CategoryModel.RemoveRecipe(recipe.RecipeModel);
    }

    /// <summary>
    /// A read-only collection of this category's recipes
    /// </summary>
    public ReadOnlyObservableCollection<RecipeViewModel> Recipes
    {
        get { return new ReadOnlyObservableCollection<RecipeViewModel>(recipes); }
    }


    private void baseCategoryDirectRecipesCollectionChanged(object sender, NotifyCollectionChangedEventArgs e)
    {
        switch (e.Action)
        {
            case NotifyCollectionChangedAction.Add:
                var recipeVM = new RecipeViewModel((Recipe)e.NewItems[0], this);
                recipes.AddIfNewAndNotNull(recipeVM);
                break;
            case NotifyCollectionChangedAction.Remove:
                recipes.Remove(new RecipeViewModel((Recipe)e.OldItems[0]));
                break;
            default:
                throw new NotImplementedException();
        }
    }

    /// <summary>
    /// Compares whether this object wraps the same Category as the parameter
    /// </summary>
    /// <param name="obj">The object to compare equality with</param>
    /// <returns>True if they wrap the same Category</returns>
    public override bool Equals(object obj)
    {
        var comparedCat = obj as CategoryViewModel;
        if(comparedCat == null)
        {return false;}
        return CategoryModel == comparedCat.CategoryModel;
    }

    /// <summary>
    /// Gets the hashcode of the wrapped Categry
    /// </summary>
    /// <returns>The hashcode</returns>
    public override int GetHashCode()
    {
        return CategoryModel.GetHashCode();
    }
}

I won't bother showing the Models (Recipe and Category) unless requested, but they basically take care of the business logic (for instance adding a recipe to a category will also add the other end of the link, i.e. if a category contains a recipe, then the recipe is also contained in that category) and basically dictate how things go. The ViewModels provide a nice interface for WPF databinding. That's the reason for the wrapper classes

Since the infinite loop is in the constructor and it's trying to create new objects, I can't just set a boolean flag to prevent this because neither object ever gets finished being constructed.

What I'm thinking is having (either as a singleton or passed in to the constructor or both) a Dictionary<Recipe, RecipeViewModel> and Dictionary<Category, CategoryViewModel> that will lazy-load the view models, but not create a new one if one already exists, but I haven't gotten around to trying to see if it'll work since it's getting late and I'm kinda tired of dealing with this for the past 6 hours or so.

No guarantee the code here will compile since I took a bunch of stuff out that was unrelated to the problem at hand.

Answer 1

Back to your original question (and the code). If what you want is to have a many-2-many relation that is automatically synchronized, then read on. The best place to look for a complex code that handles these cases is the source code of any ORM framework and it is very common problem for this domain of tools. I would look at the source code of nHibernate (https://nhibernate.svn.sourceforge.net/svnroot/nhibernate/trunk/nhibernate/) to see how it implements collections that handle both 1-N and M-N relations.

Simple something you can try is to create your own little collection class that just takes care of that. Below I removed your original wrapper classes and added a BiList collection, which is initialized with the object (the owner of the collection) and the name of the other side of the property to keep in sync with (works only for M-N, but 1-N would be simple to add). Of course, you would want to polish the code:

using System.Collections.Generic;

public interface IBiList
{
    // Need this interface only to have a 'generic' way to set the other side
    void Add(object value, bool addOtherSide);
}

public class BiList<T> : List<T>, IBiList
{
    private object owner;
    private string otherSideFieldName;

    public BiList(object owner, string otherSideFieldName) {
        this.owner = owner;
        this.otherSideFieldName = otherSideFieldName;
    }

    public new void Add(T value) {
        // add and set the other side as well
        this.Add(value, true);
    }

    void IBiList.Add(object value, bool addOtherSide) {
        this.Add((T)value, addOtherSide);
    }

    public void Add(T value, bool addOtherSide) {
        // note: may check if already in the list/collection
        if (this.Contains(value))
            return;
        // actuall add the object to the list/collection
        base.Add(value);
        // set the other side
        if (addOtherSide && value != null) {
            System.Reflection.FieldInfo x = value.GetType().GetField(this.otherSideFieldName);
            IBiList otherSide = (IBiList) x.GetValue(value);
            // do not set the other side
            otherSide.Add(this.owner, false);
        }
    }
}

class Foo
{
    public BiList<Bar> MyBars;
    public Foo() {
        MyBars = new BiList<Bar>(this, "MyFoos");
    }
}

class Bar
{
    public BiList<Foo> MyFoos;
    public Bar() {
        MyFoos = new BiList<Foo>(this, "MyBars");
    }
}



public class App
{
    public static void Main()
    {
        System.Console.WriteLine("setting...");

        Foo testFoo = new Foo();
        Bar testBar = new Bar();
        Bar testBar2 = new Bar();
        testFoo.MyBars.Add(testBar);
        testFoo.MyBars.Add(testBar2);
        //testBar.MyFoos.Add(testFoo); // do not set this side, we expect it to be set automatically, but doing so will do no harm
        System.Console.WriteLine("getting foos from Bar...");
        foreach (object x in testBar.MyFoos)
        {
            System.Console.WriteLine("  foo:" + x);
        }
        System.Console.WriteLine("getting baars from Foo...");
        foreach (object x in testFoo.MyBars)
        {
            System.Console.WriteLine("  bar:" + x);
        }
    }
}