Pull to refresh

Comments 41

Что интересно, решарпер в этом случае нас не предупредил об опасности
Странно, он в таких случаях должен ругаться на «Possible multiple enumeration of IEnumerable» и предлагать его сконвертировать в список или массив. Думаю, стоит завести баг у них в трекере.
Вот простой пример — var lines = File.ReadLines(«data.txt»);
Как раз ReadAllLines возвращает string[], который можно безопасно итерировать сколько угодно раз. Стоит подобрать пример получше.
Вы говорите абсолютно верно — ReadAllLines возвращает массив, но я пишу про другой метод — ReadLines.
Прошу извинить, проглядел.
Нужно прочувствовать функциональную парадигму, и не будет проблем.

Вместо
void Censure(IEnumerable<Bar> bar);
...
Censure(items);


надо писать
IEnumerable<Bar> Censure(IEnumerable<Bar> bar) {
    return bar.Select(x => new Bar { ACount=x.ACount, Value = (x.ACount > 1) ? "Censored" : x.Value });
}
...
items = Censure(items);
Я вот такого очень боюсь:
var bars = GeBarstFromKamchatkaDbInstance()
var censored = Censure(bars);
foreach(var current in censored)
{
    Print(current);
}
PrintFooter(curent.Count())


Как исключить риск подобной ошибке с применением функциональной парадигмы? И что в первом примере концептуально не соответствует функциональной парадигме?
В классическом LISP ф-ция (length) вообще имеет сложность O(длина-списка).
Поэтому для списка-с-длиной нужна конструкция (длина, содержимое). Или ICollection<> в .net

Видимо, в этом действительно есть проблема функциональных подходов: надо понимать, какая абстракция в какой момент уместна, копипаста плохо работает, индусов не наймёшь.
var bars = GeBarstFromKamchatkaDbInstance().ToArray()

Кстати, если мы используем asp.net mvc и у нас connection создается на время жизни контроллера, то если мы во VIew передадим такой IEnumerable, то при рендеринге получим исключение о закрытом соединении/сессии и т.д… Отсюда мораль — при передаче во View всегда делать .ToArray() или .ToList().
Ой, а я специально в mvc4 во вью отдаю IEnumerable полученный из EF без конвертации в лист/массив, чтобы не плодить в памяти сразу 100500 объектов, а дать шанс сборщику мусора убивать «пройденные». DbContext создается в конструкторе и закрывается в Dispose.

Работает, су** уже год. Что я делаю не так?
DbContext создается в конструкторе и закрывается в Dispose
действительно, Вы правы.

А теперь скажу далее, что в таком случае может быть другая ошибка, которая по русски звучит как-то так:
«На одно соединение нельзя создавать несколько DataReader-ов», такое может происходить когда вы в foreach (var item in dbLinqItems); пытаетесь еще как-то пройтись по той же коллекции или берете объект и у него начинаете foreach-чить другую связную коллекцию.
Было такое?
Было, и здесь стоит «двойная защита». Во-первых здесь «внешний» список все же считываю сразу в память и уже бегу по нему. Во вторых в продакшне включил MultipleRecordset или как-то так (позволяет более одного ридера одновременно в ms sql). Соответственно на девелопе/тесте «должно свалиться» если где-то оставляю два ридера, а продакшн отработает если проглядел при тестировании.
И держите тем самым соединение из пула соединений с БД до того момента пока не отработает GC? Отличный план. Вам везёт, потому что эти объекты не успевают покинуть nursery и собираются после обработки запроса. Но может и не повезти, и они окажутся во втором поколении, откуда их GC извлечёт ой как не скоро. Нагрузка на ваше приложение в запросах в секунду какая примерно?
Нагрузка копеечная и даже меньше. Однако за 23часа при GC.CollectionCount(0) = 2200 имею CollectionCount(1) = 1800 вместо «нормального» соотношения 10-к-1… (gen2 «нормально» — 179).

Я рассчитывал что раз Controller реализует IDisposable то этим кто-то пользуется и не проверял… Разве нет?

А о каком количестве объектов тогда идет речь? Считанные из БД объекты будут нужны до конца запроса в любом случае — они нужны view. Создание проксей и lazy load можно отключить (не уверен что отключаю везде, но по максимуму старался) настройками DbContext. Остается сам коннект и его «дерево» вспомогательных объектов? Вряд ли там объектов сравнимо с количеством объектов считанных данных и всей остальной инфраструктуры mvc…
Я рассчитывал что раз Controller реализует IDisposable то этим кто-то пользуется и не проверял… Разве нет?
А вы в Dispose у контроллера вызываете Dispose у DbContext-а?
А о каком количестве объектов тогда идет речь?
А это не важно, сколько объектов, они дешёвые. Но пока «жив» открытый с ридером DbContext, висит и TCP-соединение с базой. Причём висит не в пуле ожидая, когда кому-то ещё понадобится, а висит неиспользуемым. И вот это уже проблема. Если его вовремя не диспозить, то у вас под нагрузкой могут полезть проблемы в связи с исчерпанием доступных соединений к базе и веб-приложение встанет раком.
А вы в Dispose у контроллера вызываете Dispose у DbContext-а?

Конечно, все в лучших традициях msdn-а.

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


Не, эти грабли уже давно собраны.

К сожалению рака в повальном выживании Gen0 придется искать дальше :(
Резюмируя: ради копеечной экономии(ну не уедет во view сразу 100500 объектов, оно этому view даром не надо столько, ну не отобразить столько сущностей на экране, реальность гораздо скромнее) раскладываете себе все ниже/вышеописаные грабли которые потом старательно и аккуратно обходите. Или не обходите, по-разному выходит.

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

ИМХО это и есть ответ на «что я делаю не так?».
Да вот и нет.

Вью разные бывают и пусть и не 100500, но таблицу из 800 строк без пейджинга (не потому что лень а потому что так надо) отображать приходится.

Плюс не каждый вью пользователю на экран выводится — генерацию экспортных файлов часто удобнее через RazorEngine завернуть.

Так что размер экономии в каждом случае оценивается отдельно.

Что касается «граблей» — я не считаю граблями возможность пораньше диспозить ресурсы или необходимость вдумчивао относиться к открытым ридерам. Да и с отладкой — не припомню случая когда мне надо было подсмотреть выборку «целиком» именно в режиме отладки — она ж следом мне на экран выведется, там все и видно. А если с каким отдельным объектом «проблемы» — ну так внутри цикла я именно на него и попаду при ошибке.
Кстати, Решарпер часто лезет с ненужными советами поменять тип входной переменной с IReadOnlyList / IList на IEnumerable. Если видит, что внутри метод работает как с IEnumerable (допустим, один foreach). Не смотря на это, требуется обозначить, что на входе нужна именно конечная коллекция, а не перечислитель, с неизвестными характеристиками длины. Таким советам Решарпера лучше не следовать.
С вашим вторым примером все куда проще — не надо мешать функциональщину с мутабельными типами.
А что Вы называете функциональщиной? LINQ? Тогда я просто не могу её не мешать с мутабельными типами. Я занимаюсь разработкой корпоративных приложений и по этому большинство классов с которыми я имею дело — сущности. А они по определению мутабельны. И конечно же к ним полно LINQ запросов…

Кстати, я не говорю что в примерах написал идеальный код. единственная проблема которого — злобный IEnumerable :)
Я имел в виду не LINQ вообще, а deferred выполнение. Про мутабельность я, на самом деле, тоже некорректно выразился — дело не в типах, а в сторонних эффектах вообще (т.е. в мутабельности global state) — LINQ-выражения должны быть либо side-effect free, либо нужно гарантировать, что deferred execution произойдет только один раз — например, немедленным преобразованием в коллекцию.
Вы не понимаете смысла IEnumerable.
Это sequence причем вероятно генерируемая. Изменять только что сгенерированные сущности — бессмысленно. Они же потеряются.
Вам выше уже написали как сделать (вместо изменения создавать копии с измененными полями и их возвращать).

Про мутабельность сущностей — это вообще бред. Я сколько лет уже занимаюсь разработкой, и не испытывал никогда (кроме геймдева, и то только из за GC) проблем с immutable типами. Во многих функциональных языка вообще нету mutability.
Изменять только что сгенерированные сущности действительно не оптимально. Но почему вы сузили круг вопросов до только что сгенерированных? Я достал сущностями из базы, написал ещё пару дополнительных Where и пошел делать над ними операции.

Entity — в DDD по определению мутабельна. Почитайте Эванса. Может быть вы сущностями называете все на свете классы?
большинство классов с которыми я имею дело — сущности. А они по определению мутабельны.

А вот кстати, интересно…
Если я взял сущность, и сделал ей one.Value = "<censored>";
Разве ORM не должен при повторной выборки сущности найти по ключу в контексте изменённую сущность и отдать именно её в новых выборках? А если операция ещё и подразумевает изменение других сущностей и заканчивается Save контексту, можно незаметно подпортить данные таким присваиванием.
По-моему, все пункты высосаны из пальца: с IEnumerable возникают проблемы только тогда, когда автор кода не понимает смысл данного контракта и не более того. Это всего-лишь показатель того, что нам разрешено перебирать элементы конкретного набора элементов. Никто не обещает ни сложности данного перебора, ни тем более того, что метод, возвращающий такой набор, будет на самом деле всегда возвращать разные… Я молотком тоже не пытаюсь закручивать саморезы, хотя это и возможно.
Пункты вы имеете ввиду пункты статьи или примеры багов?
Повсеместное использование в сигнатурах методов IEnumerable нарушает принципы программирования по контракту и ведет к ошибкам.

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

var lines = File.ReadLines("data.txt");
string lastLine = lines.ElementAt(lines.Count());

Говорит только о том, что здесь явно не IEnumerable нужен. ElementAt — это просто не родная операция для него. Если решили использовать Count() или ElementAt() — то скорее контракт нарушен, когда заводили переменную/параметр типа IEnumerable
Мне кажется, IEnumerable здесь подходит вполне, например, как lines.Last()
Если нужен индексный доступ, то по смыслу самому List (в смысле вектор для плюсовиков) подходит явно. Конечно, на любых переборах, не зависимо от структуры данных программист может найти по индексу элемент. И не должен задумываться, что там за структура.

Но и интерфейсы тоже не говорят, что за структуры на самом деле используются. Поэтому тут дело не в оптимизации. Если программист в методе решил использовать индексный доступ, то он декларирует это типом переменной.
Нет, я хотел сказать, что задачи поиска последней строки IEnumerable вполне достаточно.
Сложная тема. Последняя строка как по мне — недостаточно. Поиск последней строки — это тоже индексный поиск. Также Count() — неродная операция.

У интерфейсов есть назначение. IEnumerable нужен только для перебора, возможно бесконечных множеств. Если программист объявляет такой тип, он это и декларирует — ему надо перебрать, ему не важен порядок элементов (индексы) и т.д.

ElementA(), Last(), Count() — это всё экстеншин методы, они не входят в IEnumerable. По моему, типами, программист также говорит о том, что он собирается делать. Это улучшает читаемость.

Если бы C# был таким чистым функциональным языком с немутабельными типами — то особых проблем бы не было с вызовом этих методов и с итерациями. Можно бы было один список, как в Lisp использовать для всего. C# не может гарантировать в общем случае, что в множестве не изменяются элементы от вызова к вызову, поэтому не может использовать ленивые оптимизации.
А поэтому, лучше использовать правильные типы.
У IEnumerable просто даже нет последнего элемента по смыслу. Это просто множество без заданного порядка. Нет гарантии, что два перебора дадут тот же самый порядок и что один и тот же элемент будет в конце.
Так что же должен возвращать ReadLines.
Некий IFiniteEnumerable, гарантирующий конечность последовательности?
А если открыт файл /dev/random?

Мне кажется, это уже специфика задачи (конечность файла «data.txt») поверх готовых абстракций.
Не будем же мы вводить новый класс массива, если по логике приложения в этот массив попадают только чётные числа.
То есть, полной семантики нет в каждой строке программы, надо смотреть и её окружение.
Инженерные дисциплины — не точные науки. Здесь мы выбираем разумный компромисс. В большинстве задач действительно допустимо считать IEnumerable конечным. Но вот если мы начинаем работать с ним как с массивом — тогда у нас могут быть проблемы.

Причем я говорю не о том, что проблемы могут быть «вообще»(вообще они могут быть и если считать IEnumerable конечным), а о том, что я неоднократно сталкивался с ними в реальных проектах.

По этому вызвать Last() здесь, на мой взгляд, допустимо. А вот lines.ElementAt(lines.Count()) — нет т.к. это две енумерации, и мы здесь предполагаем не только конечность, но и дешевизну енумераций.
гарантирующий конечность (и только чтение), насколько я понимаю, этот:
msdn.microsoft.com/en-gb/library/hh881542.aspx

Но вообще, действительно, надо выбирать компромисс. Last() — не является родной операцией для IEnumerable(), но при этом, бессмысленно для одного Last() делать какое-то копирование в массив. Там, внутри Last тоже делается свое либо приведение либо какие-то оптимизации, чтобы находить последний элемент без приведения.

Добавить своих расширяющих методов можно. Часто передаются IEnumerable<> кругом. Но при этом, если программисты пользуются в своим коде List<>. Чтобы не идти на компромиссы, выбирая, что же надо передавать кругом — связный список (IEnumerable<>) или список с индексным доступом(IList<>), а ToList() делает копию, что бывает накладно, то можно добавить такой метод:
IReadOnlyList<> ToReadOnlyListByRefOrCopy(...)
А там проверять, если это список, то приводить просто по ссылке и возвращать враппер, если нет — враппер над копией.
Таким образом получите некий компромисс между производительностью и выразительностью кода. Как только нужен индексный доступ — вызвали этот метод.

Типами можно хорошо описывать намерения. Не всегда всё гладко получается и с типизацией проблемы, но имхо, лучше уже так, чем просто беспорядочно IEnumerable<> использовать.
>> Там, внутри Last тоже делается свое либо приведение либо какие-то оптимизации, чтобы находить последний элемент без приведения.

А тут всё просто, очень многие Linq методы начинаются с

var asList = source as IList;
if (asList != null)
//…
else

А насчёт метода более чем согласен. Главное с постфиксом вроде того что вы предложили. Текущие .ToList/ToArray каждый раз копируют список/массив независимо от того что ты им передал. Мне кажется ребята из MS не предполагали что люди будут использовать эти методы на IEnumerable для защитной материлизации итератора (ну чтобы 2 раза не материализовать путем итерации по нему если он ещё не материализован).

Таким образом даже методы а-ля
ToListIfNeeded/ToArrayIfNeeded или ToListByRefOrCopy/ToArrayByRefOrCopy
очень хорошо впишутся в текущий LINQ зоопарк. И по реализации (они тоже принмают IEnumerable, но всё равно проверяют на тип), и по полезности (сэкономят кучу памяти когда IEnumerable уже материализован.

Хотя я у себя в проектах обычно пишу простенький
IList MaterializeIfNeeded(IEnumerable source, int exactCount = -1)
Который делает то же самое, только конкретный возвращаемый тип не указан и есть exactCount на тот нечастый случай если ты знаешь кол-во элементов в итераторе, тогда материализация пройдёт без лишней памяти (там просто листу будет дан сразу правильный Capacity)
Это дырка в абстракциях. Не будьте наивным. В MSFT сидят обычные люди и они налажали, если судить с позиции строгих абстракций. Это нормально. Коллекции вообще дырявые, хотя в 4.5 это ненмого подправили, но сколько лет понадобилось.

А что касается ваших ожиданий от List. Если файл гигабайт 10 — то с листом ваш код быстро пойдёт отдыхать, а вот IEnumerable ничего никуда не складирует, и всё будет окей с его Last()

Ваша задача как программиста не пытаться подстроить своё мышление под говноабстракции придуманные MS, а выбрать/написать абстракции под ваше красивое и эффективное решение (не складирующее строки файла в памяти).
И IEnumerable — почти отлично подходит для поиска последней строки. Г-н qw1 снизу привел вообще отличный вариант интерфейса.
Если хотите закрыть дырку в абстракциях — можете сделать Extension метод .MarkFinite на получении списка строк и чтобы он возвращал IFiniteEnumerable который унаследован от IEnumerable. Зато весь код дальше сможет делать +1 предположение касательно итератора.
Прошу прощения за косноязычность. Конечно, я не подразумевал, что ради единственного Last() надо кругом всё превращать в массивы. Ответ немного выше.
Но снова об оптимизации. Зло это. Вообще, думать об оптимизации. Я со всем согласен и с Вами и с постом, не нравится только ракурс — рассматривание проблемы с т.з. того, как это плохо — многократные итерации. И Ваша т.з. — как это плохо — складирование строк в памяти. Как вроде ничего больше в программировании интересного и нет )
Ну примерно об этом с и постарался рассказать. Вы привели первый абзац выводов. А во втором как раз и говорится, что если необходим перебор элементов и не более — то это и есть единственно допустимый случай использования IEnumerable.

По поводу частоты использования — мне, к сожалению, в реальных проектах чаще встречаются «неправильные» использования IEnumerable. А подсказка решарпера, о которой говорил hVostt сильно подливает масла в огонь.
>>Но в реальных программах, чаще всего, программист думает об IEnumerable как о коллекции. К примеру, даже появился такой
>>паттерн — защитная копия IEnumerable. Т.е. вызов ToArray() в начале метода, когда туда приходит IEnumerable

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

А про паттерн — можно поподробнее? В каких случая такое может быть нужно?
Вам повезло, что вы и ваши коллеги всегда с рождения пишут идеальный код;)

Последняя ссылка в посте.
Могу порекомпндовать для немодифицируемыъ списков библлиотеку Immutable Collections blogs.msdn.com/b/bclteam/archive/2013/03/06/update-to-immutable-collections.aspx
А насчёт Count для IEnumerable я точно знаю что реализирована Generic специализация для List которая вызовёт именно IList метод Count вместо обобщённого, а чтобы не было проблем с LINQ to SQL смотреть всегда что генерируется на вход SQL-сервера, очень часто даже простая смена порядка накладывания критериев выборки.
Sign up to leave a comment.

Articles