Как я могу избежать этого бесконечного цикла? - PullRequest
2 голосов
/ 25 мая 2009

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

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

Следующие 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 часов или около того.

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

Ответы [ 8 ]

2 голосов
/ 29 мая 2009

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

Решение:

Ваш код не работает в следующих местах:

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

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

option-1: использовать простой а-ля Factory Pattern для создания ваших объектов, но также отслеживать их:

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 не сможет очистить их от памяти.

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

2 голосов
/ 25 мая 2009

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

Простое, что вы можете попробовать, это создать свой собственный небольшой класс коллекции, который просто позаботится об этом. Ниже я удалил ваши исходные классы-обертки и добавил коллекцию BiList, которая инициализируется объектом (владельцем коллекции) и именем другой стороны свойства, с которой нужно синхронизироваться (работает только для MN, но 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);
        }
    }
}
1 голос
/ 25 мая 2009

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

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

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

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

1 голос
/ 25 мая 2009

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

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

1 голос
/ 25 мая 2009

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

1 голос
/ 25 мая 2009

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

1 голос
/ 25 мая 2009

Параметры:

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

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

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

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

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

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

Заключение

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

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

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

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

Добавление

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

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

  • Я думаю, что коллекции категорий и рецептов в классах модели представления являются избыточными
    • у вас уже есть информация M: M в базовом объекте категории, так зачем дублировать ее в модели представления
    • похоже, что обработчики, измененные коллекцией, также вызовут бесконечную рекурсию
    • обработчики, изменяющие коллекцию, по-видимому, не обновляют базовую информацию M: M для упакованного рецепта / категории
  • Я думаю, что цель модели представления состоит в том, чтобы представить базовые данные модели, а не индивидуально оборачивать каждый из ее компонентов.
    • Это кажется избыточным и нарушением инкапсуляции
    • Это также источник вашей проблемы бесконечной рекурсии
    • Наивно, я бы ожидал, что свойства ObservableCollection просто вернут коллекции базовой модели ...

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

ObservableCollection<Game>

а не

ObservableCollection<GameWrapper>
0 голосов
/ 25 мая 2009

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

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

Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...