Как я могу избежать этого бесконечного цикла?

Такое ощущение, что для этого должно быть какое-то полупростое решение, но я просто не могу его понять.

Редактировать: предыдущий пример показал бесконечный цикл более четко, но это дает немного больше контекста. Ознакомьтесь с предварительным редактированием для быстрого обзора проблемы.

Следующие 2 класса представляют модели представления модели представления модели (MVVM) узор.

/// <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();
    }
}

Я не буду показывать модели (рецепт и категорию), если это не требуется, но они в основном заботятся о бизнес-логике (например, добавление рецепта в категорию также добавит другой конец ссылки, т. е. если категория содержит рецепт, то рецепт также содержится в этой категории) и в основном определяют, как все пойдет. ViewModels предоставляют удобный интерфейс для привязки данных WPF. Это причина для классов-оболочек

Поскольку бесконечный цикл находится в конструкторе, и он пытается создать новые объекты, я не могу просто установить логический флаг, чтобы предотвратить это, потому что ни один объект никогда не завершается созданием.

Я имею в виду наличие (либо в виде синглтона, либо в конструкторе, либо в обоих случаях) Dictionary<Recipe, RecipeViewModel> и Dictionary<Category, CategoryViewModel>, которые будут лениво загружать модели представления, но не создавать новую, если она уже существует, но я этого не сделал. удосужился попытаться посмотреть, сработает ли это, так как уже поздно, и я немного устал иметь дело с этим в течение последних 6 часов или около того.

Нет гарантии, что код здесь скомпилируется, так как я вытащил кучу вещей, которые не имели отношения к рассматриваемой проблеме.


person Davy8    schedule 25.05.2009    source источник


Ответы (8)


Чувак, мой ответ не такой крутой, как те, что у DI. Но...

Проще говоря, я думаю, вы должны создать свои оболочки, прежде чем начинать их связывать. Пройдитесь по всему списку Foos, создав FooWrappers. Затем пройдитесь по Bars и создайте BarWrappers. Затем прочитайте исходный Foos, добавив соответствующие ссылки BarWrapper на MyBarWrappers в связанном FooWrapper, и наоборот для Bars.

Если вы настаиваете на создании оболочки для экземпляра Foo и немедленном создании отношений с каждым из его экземпляров Bar, тогда вы должны «разорвать» цикл, отметив, над каким Foo вы работаете, т. е. Foo_1, и позволить каждому из экземпляров BarWrapper НЕ ЗНАЕТЕ создавать еще один экземпляр FooWrapper_1 внутри своей коллекции MyFooWrappers. Ведь вы, по сути, уже создаете FooWrapper_1 выше (или как бы ниже) стека вызовов.

Итог: с точки зрения разумности кода конструкторы-оболочки не должны создавать больше оболочек. В лучшем случае он должен только знать/обнаруживать, что единственная уникальная оболочка существует где-то еще для каждого Foo и Bar, и МОЖЕТ создавать оболочку ТОЛЬКО, если она не находит ее в другом месте.

person Jeff Meatball Yang    schedule 25.05.2009
comment
Да, я думал о том, чтобы иметь где-нибудь словарь с Foo в качестве ключей и FooWrappers в качестве значений. Слишком устал, чтобы попробовать это сегодня вечером. - person Davy8; 25.05.2009

Вернемся к исходному вопросу (и коду). Если вы хотите иметь отношение «многие-2-многие», которое автоматически синхронизируется, читайте дальше. Лучшее место для поиска сложного кода, который обрабатывает эти случаи, — это исходный код любой инфраструктуры ORM, и это очень распространенная проблема для этой области инструментов. Я бы посмотрел исходный код nHibernate (https://nhibernate.svn.sourceforge.net/svnroot/nhibernate/trunk/nhibernate/), чтобы увидеть, как он реализует коллекции, которые обрабатывают отношения 1-N и M-N.

Вы можете попробовать просто создать свой собственный небольшой класс коллекции, который просто позаботится об этом. Ниже я удалил ваши исходные классы-оболочки и добавил коллекцию BiList, которая инициализируется объектом (владельцем коллекции) и именем другой стороны свойства для синхронизации (работает только для M-N, но 1- N было бы просто добавить). Конечно, вы хотели бы отшлифовать код:

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);
        }
    }
}
person van    schedule 25.05.2009

Прежде всего, DI не решит вашу проблему, но одна вещь, всегда связанная с DI, которая решит вашу проблему, — это использование Container (или контекст с возможностью поиска)

Решение:

Ваш код не работает в этих местах:

var catVM = new CategoryViewModel(cat); //Causes infinite loop
...
var recipeVM = new RecipeViewModel(item); //Causes infinite loop

Проблема вызвана тем, что вы создаете оболочку (xxxViewModel) для объекта, даже если он уже существует. Вместо того, чтобы снова создавать оболочку для того же объекта, вам нужно проверить, существует ли уже обертка для этой модели, и использовать ее вместо нее. Итак, вам нужен Контейнер для отслеживания всех созданных объектов. Ваши варианты:

вариант 1: используйте простой фабричный шаблон для создания своих объектов, но также отслеживайте их:

class CategoryViewModelFactory
{
    // TODO: choose your own GOOD implementation - the way here is for code brevity only
    // Or add the logic to some other existing container
    private static IDictionary<Category, CategoryViewModel>  items = new Dictionary<Category, CategoryViewModel>();
    public static CategoryViewModel GetOrCreate(Category cat)
    {
        if (!items.ContainsKey(cat))
            items[cat] = new CategoryViewModel(cat);
        return items[cat];
    }
}

Затем вы делаете то же самое на стороне рецепта, и проблемный код исправлен:

  // OLD: Causes infinite loop
  //var catVM = new CategoryViewModel(cat);
  // NEW: Works 
  var catVM = CategoryViewModelFactory.GetOrCreate(cat);

Осторожно: возможны утечки памяти?

Одна вещь, о которой вы должны знать (и именно поэтому вам не следует использовать реализацию dummy a-la factory), заключается в том, что эти объекты creator будут хранить ссылки на как объекты модели, так и их оболочки View. Поэтому GC не сможет очистить их из памяти.

вариант 1a: скорее всего, в вашем приложении уже есть контроллер (или контекст), к которому представления имеют доступ. В этом случае вместо создания этих а-ля фабрик я бы просто переместил методы GetOrCreate в этот контекст. В этом случае, когда контекст исчезнет (форма закрыта), ссылки на эти словари также будут удалены, и проблема с утечкой исчезнет.

person van    schedule 28.05.2009

Я бы рекомендовал вам избавиться от взаимной зависимости, например, с помощью принципа инверсии зависимостей, http://en.wikipedia.org/wiki/Dependency_inversion_principle -- по крайней мере одна из двух сторон Foo и Bar (или их обертки) зависят от абстрактного интерфейса, который реализует другая сторона, а не от двух конкретных классов, напрямую зависящих от друг друга, что может легко привести к кошмарам циклической зависимости и взаимной рекурсии, подобным тому, который вы наблюдаете. Кроме того, существуют альтернативные способы реализации отношений «многие ко многим», которые, возможно, стоит рассмотреть (и которые, возможно, легче подвергнуть инверсии зависимости за счет введения подходящих интерфейсов).

person Alex Martelli    schedule 25.05.2009
comment
В частности, Google Guice быстро справляется с такими вещами. См. code.google.com/p/google-guice. - person Dave W. Smith; 25.05.2009
comment
Я читаю PDF-файл Роберта Мартина, связанный с вики, прямо сейчас, примерно на полпути, и хотя я согласен с тем, что это хорошая практика, я не понимаю, как наличие интерфейсов IFoo и IBar поможет в этом конкретном сценарии. Хотя я могу что-то упустить. - person Davy8; 25.05.2009
comment
К сожалению, каким бы крутым ни был DI, он не поможет с данной структурой. - person Steven A. Lowe; 25.05.2009

Я собираюсь сказать шаблон Factory. Таким образом, вы можете конструировать каждый по очереди, затем добавлять их друг к другу, затем возвращать их всех скрытыми от посторонних глаз заводом.

person JP Alioto    schedule 25.05.2009

Это напоминает мне, как сериализация предотвращает бесконечные циклы, когда объекты содержат другие объекты. Он сопоставляет хэш-код каждого объекта с его массивом байтов, поэтому, когда объект содержит ссылку на другой объект, он: а) не сериализует один и тот же объект дважды и б) не сериализует себя в бесконечный цикл.

У вас по сути такая же проблема. Решение может быть таким же простым, как использование какой-либо карты вместо коллекции списков. Если то, что вы получаете, это «многие ко многим», вы просто создаете карту списков.

person dviljoen    schedule 25.05.2009

опции:

  1. реализовать тест на членство, например. проверьте bar-is-member-of-foo перед добавлением
  2. переместить отношение "многие ко многим" в свой собственный класс

последний предпочтительнее, я думаю - это более реляционно звучит

конечно, в примере с foo-bar мы действительно не знаем, какова цель, поэтому ваш пробег может отличаться

РЕДАКТИРОВАТЬ: учитывая код в исходном вопросе, № 1 не будет работать, потому что бесконечная рекурсия происходит до того, как что-либо будет добавлено в любой список.

Есть несколько проблем с этим подходом/вопросом, вероятно, потому, что он был абстрагирован до почти глупости - хорошо для иллюстрации проблемы кодирования, но не так хорошо для объяснения исходного намерения/цели:

  1. классы-оболочки на самом деле ничего не обертывают и не добавляют никакого полезного поведения; это мешает понять, зачем они нужны
  2. с данной структурой вы не можете инициализировать списки в конструкторе вообще, потому что каждый список-оболочка немедленно создает новый экземпляр другого списка-оболочки
  3. даже если вы отделяете инициализацию от построения, у вас все еще есть циклическая зависимость со скрытым членством (т.е. оболочки ссылаются друг на друга, но скрывают элементы foo/bar от проверки содержимого; что на самом деле не имеет значения, потому что код никогда не добавляет что-либо в любом списке!)
  4. прямой реляционный подход будет работать, но требует механизмов поиска и предполагает, что оболочки будут создаваться по мере необходимости, а не заранее, например. массив с функциями поиска или пара словарей (например, Dictionary>, Dictionary>) подойдет для сопоставления, но может не соответствовать вашей объектной модели

Вывод

Я не думаю, что структура как описано будет работать. Ни с DI, ни с фабрикой, ни вообще — потому что обертки ссылаются друг на друга, скрывая подсписки.

Эта структура намекает на невысказанные неверные предположения, но без контекста мы не можем определить, какими они могут быть.

Пожалуйста, переформулируйте проблему в исходном контексте с реальными объектами и желаемой целью/намерением.

Или, по крайней мере, укажите, какую структуру, по вашему мнению, должен создавать ваш пример кода, который должен создавать. ;-)

Приложение

Спасибо за разъяснение, это проясняет ситуацию.

Я не работал с привязкой данных WPF, но просмотрел эту статью MSDN - поэтому следующее может быть или не быть полезным и/или правильным:

  • I think the categories and recipes collections in the view-model classes are redundant
    • you already have the M:M information in the underlying category object, so why duplicate it in the view-model
    • похоже, что ваши обработчики изменения коллекции также вызовут бесконечную рекурсию
    • кажется, что обработчики с измененной коллекцией не обновляют базовую информацию M:M для обернутого рецепта/категории.
  • I think the purpose of the view-model is to expose the underlying model data, not to indivudally wrap each of its components.
    • This seems redundant and a violation of encapsulation
    • Это также источник вашей проблемы с бесконечной рекурсией.
    • Наивно я ожидал, что свойства ObservableCollection просто вернут коллекции базовой модели...

Структура, которую вы имеете, представляет собой представление «перевернутого индекса» отношения «многие ко многим», что довольно часто используется для оптимизированного поиска и управления зависимостями. Это сводится к паре отношений «один ко многим». Посмотрите на пример GamesViewModel в статье MSDN — обратите внимание, что свойство Games — это просто

ObservableCollection<Game>

и не

ObservableCollection<GameWrapper>
person Steven A. Lowe    schedule 25.05.2009
comment
Я перепробовал все варианты, которые мог найти для № 1, но не играл в кости. Проблема в том, что бесконечный цикл происходит до того, как что-то действительно добавлено. Я мог бы заменить MyBarWrappers.Add(new BarWrapper(bar)); только с новым BarWrapper(bar); и то же самое для другого, и переполнение стека все равно произойдет. - person Davy8; 25.05.2009
comment
# 2 кажется, что это, вероятно, сработает, но не выглядит очень объектно-ориентированным и больше похоже на стиль реляционной БД. Концептуально я не понимаю, почему это так сложно, все, что я хочу, это отразить отношения в объектах-оболочках, которые у меня есть в бизнес-объектах. - person Davy8; 25.05.2009
comment
На самом деле, если я не ошибаюсь, что вполне возможно, № 2 тоже не работает. Я создал в основном класс кортежа FooBarWrapperPair, и вместо того, чтобы FooWrapper содержал список BarWrapper, у меня есть FooWrapper, содержащий список FooBarWrapperPairs. Но проблема в том, что мне все еще нужно создать BarWrapper, чтобы поместить его в пару. Хотя я могу неправильно понять ваш смысл. - person Davy8; 25.05.2009
comment
@[Davy8]: см. правки; обратите внимание, что без контекста это кажется очень странным - классы-оболочки не служат никакой очевидной цели - person Steven A. Lowe; 25.05.2009
comment
@[Davy8]: редактирование завершено, извините за задержку. Я думаю, вы загнали себя в угол, и вам нужно сделать резервную копию и переосмыслить область и цели. - person Steven A. Lowe; 25.05.2009
comment
Отредактировано с реальным примером (за вычетом несвязанных деталей) и дополнительным контекстом - person Davy8; 25.05.2009
comment
В этом конкретном случае я мог бы просто иметь ObservableCollection модели напрямую, но это зависит от того факта, что моя модель реализует IPropertyChanged и ICollectionChanged и, следовательно, является привязываемой. Если бы модель была чем-то, что не реализовало их, и у меня не было исходного кода для нее, это не сработало бы, потому что, хотя ObserableCollection уведомит об изменении коллекции, каждый из элементов в коллекции должен предоставить WPF дружеское уведомление о том, что они сами изменились. - person Davy8; 25.05.2009
comment
Я попытался загрузить образец кода по ссылке, которую вы дали, чтобы посмотреть, как выглядит класс Game, поскольку он не был показан в статье, но, к сожалению, ни один из проектов не загружался в Visual C# Express. Я просмотрел некоторые файлы CS напрямую, и похоже, что Game (ну, частичный класс для Game, похоже, что другая половина — это класс, сгенерированный Entities Framework) действительно реализует INotifyPropertyChanged. Теперь это выглядит так, что представление зависит от модели, когда, если я правильно понимаю MVVM, представление должно зависеть только от модели представления, которая зависит от модели. - person Davy8; 25.05.2009
comment
Кроме того, моя RecipeViewModel имеет немного другие свойства для привязки, где в модели у меня есть свойство TimeSpan, в ViewModel оно отображается как отдельное поле Minutes и Hours для привязки к текстовым полям. - person Davy8; 25.05.2009
comment
@[Davy8]: тогда, я думаю, вам понадобится динамическая фабрика и поиск; извините, я не могу быть более полезным, я еще не беспокоился о WTF и др. Вы можете попробовать проголосовать за ответы некоторых других людей и посмотреть, потребуют ли они дополнительных усилий. ;-) - person Steven A. Lowe; 26.05.2009

Итак, Фу и Бар — Модели. Foo — это список Bars, а Bar — список Foos. Если я правильно понимаю, у вас есть два объекта, которые являются не чем иным, как контейнерами друг для друга. А — множество всех В, а В — множество всех А? Разве это не круглое по самой своей природе? Это бесконечная рекурсия по самому своему определению. Включает ли реальный случай больше поведения? Возможно, поэтому людям трудно объяснить решение.

Моя единственная мысль состоит в том, что если это действительно намеренно, то использовать статические классы или использовать статическую переменную для записи того, что классы были созданы один раз и только один раз.

person KnownIssues    schedule 25.05.2009
comment
Дело не в том, что A — это множество всех B, а B — это множество всех A. Более конкретным примером вместо Foo и Bar являются книги и библиотеки. Книга может находиться во многих библиотеках, а библиотека может содержать много книг. Книга (или, что более реалистично, запись книги в каком-либо компьютерном каталоге) имеет список всех библиотек, в которых она хранится, а библиотека имеет список всех книг, которые она содержит. - person Davy8; 25.05.2009