Древовидный список на ASP.NET MVC 4

[@lair]

А зачем? Для задачи, описанной в статье, это не нужно — там все равно выбираются сразу все элементы.

(EF вообще достаточно плохо работает с древовидными структурами, к сожалению)

[@gouranga]

А почему не nested set?

[@marshinov]

News = context.News.Where(x => !x.IsDeleted).ToArray().Select(x => new NewsModel(x))

Будет повторный проход по энумератору, лучше добавить .ToList() или ToArray().

А NewsListModel лусче сразу сделать ListModel where T:ITreeView

[@lair]

Будет повторный проход по энумератору, лучше добавить .ToList() или ToArray().

… а первый ToArray() заменить на AsEnumerable(), чтобы обойтись одной материализацией массива, да.

[@lair]

Вообще, конечно, статья ни о чем. Нигде не объясняется, зачем (например, зачем новости имеют древовидную структуру) или почему (например, почему для отображения нужна отдельная модель) что-то делается.

Ну и код, конечно, ужасающий.

.Where(c).Any(), когда можно сказать просто Any(c)?
foreach() {if}, когда можно сказать Where()?
Логика в view (в том числе — логика отбора данных)?
Partial вместо helper?

[@Revkov]

Вообще, конечно, статья ни о чем. Нигде не объясняется, зачем (например, зачем новости имеют древовидную структуру) или почему (например, почему для отображения нужна отдельная модель) что-то делается.

Тут суть не в новостях, а в работе с древовидным списком. Класс News, можно назвать по-разному, например та же запись (Record). Для отображения нужна отдельная модель, чтобы напрямую не работать с базой данных, потому что это может повлечь за собой различные проблемы. Например, нарушение целостности и потерю информации.

Ну и код, конечно, ужасающий.

.Where©.Any(), когда можно сказать просто Any©?
foreach() {if}, когда можно сказать Where()?
Логика в view (в том числе — логика отбора данных)?
Partial вместо helper?

Ваш пример тоже имеет место быть.

[@lair]

Тут суть не в новостях, а в работе с древовидным списком.

Тем не менее, примеры нужно подбирать подходящие, а не произвольные.

Для отображения нужна отдельная модель, чтобы напрямую не работать с базой данных,

Напрямую? Очень смешно. Между вами и БД как минимум три слоя: сущности EF, маппер EF, ADO.net.

потому что это может повлечь за собой различные проблемы. Например, нарушение целостности и потерю информации.

Покажите на примере, каким образом работа напрямую с (правильно спроектированной) дата-моделью поверх правильно спроектированной БД может повлечь за собой нарушение целостности и/или потерю информации. И каким образом добавление лишнего слоя от этого спасает.

Я, заметим, не спорю, что отдельная viewmodel — это правильно. Только, в отличие от вашего поста, я могу четко показать зачем она нужна (и более того, как ее введение убирает разом все мои вопросы к вашему коду представления).

[@Revkov]

Я, заметим, не спорю, что отдельная viewmodel — это правильно. Только, в отличие от вашего поста, я могу четко показать зачем она нужна (и более того, как ее введение убирает разом все мои вопросы к вашему коду представления).

1. Если работать напрямую с контекстом будет постоянное соединение с базой данных.
2. Модель будет зависеть от конкретной реализации БД.
3. Модель уже сформирована и храниться в оперативной памяти, что ускоряет работу с данными.
4. Изменяя модель вы будете, автоматически изменять базу данных, что не есть хорошо. (Утрированно конечно, для сохранения изменений нужно вызвать SaveChanges(), но мало ли, что)
Если что-то упустил и вы знаете больше, поправьте или добавьте. Интересны знания других разработчиков.

[@levchick]

1. Не будет, если закрывать контекст. Выбрали данные в блоке using и все, сущности сформированы, находятся в оперативной памяти, соединение закрыто.
2. Вот это уже весомый аргумент, не всегда те данные, которые будут отображаться в представлении один в один по структуре совпадают с БД
3. Совсем не ускоряет. Что бы сформировать модель для представления Вам так же ее надо выбрать нужные данные из БД. И сущности EF так же будут находится в оперативной памяти, если Вы их корректно извлечете из БД, закрыв после соединение (см. п.1)
4. В целом согласен, но не припомню ни разу, что бы приходилось изменять как модель для представления, так и сущность, что бы показать ее в представлении. Сформировали, а дальше просто вывод нужных данных.

[@levchick]

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

[@qw1]

На мой взгляд, это ужасный совет.

Неподгруженные ссылки и коллекции будут выкидывать System.ObjectDisposedException(«Экземпляр ObjectContext был удален и больше не может использоваться в операциях, требующих соединения.») при обращении к ним. Непонятно, что станет, если мы захотим отдать объект по WCF или записать в XML (в разных случаях нужны загруженными разные поля).

Есть шанс запутаться в экземплярах объектов из разных контекстов, когда логика начнёт усложняться. Лучше придерживаться правила — entity не должны использоваться вне своего контекста. Нужно отдать за пределы — скопируй в data object.

[@levchick]

Минуточку, я советовал лишь закрывать контекст, после выборки данных, но не призывал отказываться от ViewModel — это разве плохой совет?:) С Вашим комментарием абсолютно согласен, это как раз хороший аргумент в сторону использования ViewModel ну и data object в целом. По хорошему: открыли контекст, выбрали необходимые сущности, из них сформировали нужные модели для представления, закрыли контекст ну и дальше собственно в представление.

Просто я согласен с lair, что аргументы Revkov несколько неполные, проблемы описанные в пунктах 1,3,4 можно решить и без ViewModel, особенно в небольших проектах, но конечно это не является хорошей практикой, а вот из того, почему не является, вытекает почему стоит таки использовать отдельные объекты для передачи в представления. Как то так.

[@qw1]

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

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

[@levchick]

Согласен, но проблему из пункта 4 все же такой подход решает, хоть он и не совсем корректный.

[@lair]

Неподгруженные ссылки и коллекции будут выкидывать System.ObjectDisposedException(«Экземпляр ObjectContext был удален и больше не может использоваться в операциях, требующих соединения.») при обращении к ним.

А вы все еще используете lazy load в MVC-приложении? Жестоко… Впрочем, в WCF не лучше.

Нужно отдать за пределы — скопируй в data object.

В принципе, ничего не имею против правила, но все-таки рекомендую задумываться о том, зачем и когда это делается. Иначе мы получаем как у автора статьи — два идентичных слоя; оверхед есть, пользы нет.

[@lair]

Если работать напрямую с контекстом будет постоянное соединение с базой данных.

Вы, видимо, не понимаете, как работает EF. Соединение открывается в момент начала итерирования и закрывается по получению последнего элемента (или при выходе итератора из области видимости, или при явном закрытии контекста). Чтобы получить «постоянное» соединение, нужно очень постараться.

Модель будет зависеть от конкретной реализации БД.

Во-первых, не обязательно. А во-вторых — что в этом плохого?

Модель уже сформирована и храниться в оперативной памяти, что ускоряет работу с данными.

Не модель, а данные. И не важно, как вы их получили — из EF через ToArray, или создав по предыдущим данным новые объекты — они все равно будут в памяти, скорость не меняется. Наконец, это хорошо до тех пор, пока у вас в памяти сто объектов. Когда вы работаете с полумиллионом — памяти может и не хватить…

Изменяя модель вы будете, автоматически изменять базу данных, что не есть хорошо. (Утрированно конечно, для сохранения изменений нужно вызвать SaveChanges(), но мало ли, что)

Во-первых, SaveChanges. Во-вторых, сущности можно сразу же оторвать от контекста через NoTracking. Так что никакого «автоматически».

Если что-то упустил и вы знаете больше, поправьте или добавьте.

Вы упустили тот банальный факт, что viewmodel предназначена для работы с представлением. И полностью под него оптимизируется — как раз так, чтобы в представлении логики не было совсем или почти совсем. Но у вас этого не сделано — именно потому, что вы не видите этого предназначения; и, в свою очередь, именно поэтому этот слой у вас лишний.

[@alexanderzaytsev]

2. Модель будет зависеть от конкретной реализации БД.

От ORM, а не от БД.

У вас и так весь код на EntityFramework завязан. К чему городить лишнюю сущность?

[@Cryptochild]

.Where(c).Any(), когда можно сказать просто Any(c)?

Потому что .Where() имеет оптимизации для итерирования разных типов коллекций, а .Any() — нет.

[@qw1]

Умм, действительно! Воспользовался декомпилятором, получил

    public static bool Any<TSource>(this IEnumerable<TSource> source, Func<TSource, bool> predicate)
    {
      if (source == null)
        throw Error.ArgumentNull("source");
      if (predicate == null)
        throw Error.ArgumentNull("predicate");
      foreach (TSource source1 in source)
      {
        if (predicate(source1))
          return true;
      }
      return false;
    }
    public static IEnumerable<TSource> Where<TSource>(this IEnumerable<TSource> source, Func<TSource, bool> predicate)
    {
      if (source == null)
        throw Error.ArgumentNull("source");
      if (predicate == null)
        throw Error.ArgumentNull("predicate");
      if (source is Enumerable.Iterator<TSource>)
        return ((Enumerable.Iterator<TSource>) source).Where(predicate);
      if (source is TSource[])
        return (IEnumerable<TSource>) new Enumerable.WhereArrayIterator<TSource>((TSource[]) source, predicate);
      if (source is List<TSource>)
        return (IEnumerable<TSource>) new Enumerable.WhereListIterator<TSource>((List<TSource>) source, predicate);
      else
        return (IEnumerable<TSource>) new Enumerable.WhereEnumerableIterator<TSource>(source, predicate);
    }

Выходит, Resharper даёт вредный совет заменить .Where(fn).Any() на .Any(fn)

Хотя, учитывая, что Any — конечная конструкция (результат не итерируется), может и нормально…

[@Cryptochild]

А сама Any() не итерирует что ли? :-)

[@qw1]

Если бы внутри Where() был foreach, то конструкции .Where(fn).First() или .Where(f1).Where(f2) были бы неоптимальными. А поскольку после .Any уже ничего быть не может, её можно попроще написать.

[@lair]

Выходит, Resharper даёт вредный совет заменить .Where(fn).Any() на .Any(fn)

Не, ничего вредного. Resharper исходит из наглядности и читабельности, а внутренние оптимизации BCL всегда могут поменяться. Более того, как я писал выше, я не вижу, каким образом эти конкретные оптимизации влияют на конкретный Any.

[@lair]

И что, сильный выигрыш эта оптимизация дает на сотне объектов?

(не говоря уже о том, что на IQueryable ситуация может оказаться обратной)

[@lair]

Кстати, а как именно эти оптимизации влияют на скорость при работе с конкретным Any()? Отличия будут только для массивов, причем минимальные.

[@nile1]

Множественное число от child — children, а не childs.

[@vba]

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