Pull to refresh

Comments 46

А зачем Any, если можно сделать сразу Sum?
1. Это гипотетическая ситуация
2. Sum null не вернет
А если использовать DefaultIfEmpty, каковы плюсы и минусы?
Назначение у DefaultIsEmpty() несколько другое, этот метод возвращает перечислитель с одним дефолтным элементом в том случае если исходный перечислитель не содержит элементов. Можно применить для симуляции поведения LEFT OUTTER JOIN, но этот метод никак не применим если надо проверить наличие элементов в исходном перечислителе.
Т.е. всё это чисто ради var items = ienumerable.ToList(); if (items.Count == 0) {… } else {… }?

Вы читали статью? Ваш пример там указан как потребляющий O(N) памяти.

Бывает. Если что, любой вариант будет потреблять хотя бы O(N).

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

Зачем вы спрашиваете меня про суммирование миллиарда чисел? Конкретная задача — конкретное решение.

А вот статья как раз про общий случай. И на ровном месте мы зачем-то получаем несколько десятков строк кода, которые кому-то нужно понимать и поддерживать. Зачем — автор так и не пояснил. При том, что даже в общем случае задача решается строчки в 3 кода, а в частных (мы же практические задачи решаем, верно? А не диванные теоретики, которые программируют ради программирования) — вообще бесплатно.

Не любой.
Например, агрегаторпо 2м элементами будет хранить O(1), а Reverse() будет хранить все O(N-1).

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

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

Я, если честно, что там откомментировал, что тут напишу: какой вообще смысл отдельно проверять IEnumerable на пустоту?

Некоторые стандартные методы кидают исключение при получении пустого перечисления.

Согласен. Самый, наверное, банальный пример это items.Min() и items.Max().
какой вообще смысл отдельно проверять IEnumerable на пустоту?

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


(Так же, как не нужно и проверять коллекцию на null — это дело контракта метода, когда мы генерируем ArgumentNullException, либо позволяем сгенерироваться NRE.)


Если мы не работаем с БД или не обрабатываем какой-то специфичный случай с перебором бесконечной последовательности элементов, то в обычном коде IEnumerable используются только в Linq — там получающиеся в результате запросов коллекции на null проверять не надо, а вот вызывать Any при необходимости можно.


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


При этом создаются методы, принимающие на вход коллекцию, где неважно — массив это, список, или что то еще.
Главное — что подразумевается, что это "легкий" тип, не что-то лениво считываемое итератором из базы.
В этом случае в качестве типа аргумента как раз подходит IReadOnlyCollection(T), и там вполне может быть необходимость проверки на пустоту (поскольку это не чтение таблицы из БД), и для этого есть свойство Count.


Но часто ли вы видели использование того же IReadOnlyCollection(T)?
Как правильно, в прикладном коде создаются классы со свойствами, результатами и аргументами методов, которые в случае коллекций имеют либо "недостаточный" IEnumerable(T) (более общий, чем нужно), либо слишком конкретный тип (в последнем случае очень любят List(T) — причем сразу реализацию, а не интерфейс).


Ну и когда используют IEnumerable(T) не по назначению, то и получается, что приходится проверять коллекцию на пустоту.


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

Если это именно IEnumerable(T) или IQueryable(T), то никакого смысла проверять на пустоту нет.

Вот вполне реалистичный пример, где проверка на пустоту имеет смысл:
IEnumerable<int> items = CalculateItemsUsing3rdPartyLibabry();
Console.WriteLine(items.Max());

(можем получить исключение Sequence contains no elements)

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

Верно, спасибо за пример.

Но если разработчики CalculateItemsUsing3rdPartyLibabry предоствили API, возвращающее IEnumerable, то они должны позаботиться и о том, чтобы Linq-метод Any() отрабатывал нормальным образом применительно к их реализации.
Спасибо за пример, если честно, не знал что существует отдельная реализация int? Max(this IEnumerable<int?> source) Но по сути, проверка на пустоту все же есть, только она скрыта в реализации Max(this IEnumarable<int?> source)

Здесь по прежнему IEnumerable(int), вот в чем штука.
В Max передается selector, который задает правило преобразования в другой, прежде, чем сравнивать элементы, и тем самым задает и тип результата.


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


InvalidOperationException
source contains no elements.
Вы указали на другой метод.
А Nullable метод
Enumerable.Max<TSource>(IEnumerable<TSource>, Func<TSource, Nullable<Int32>>)
не кидает исключений если source contains no elements
Документация

Про документацию вам уже ответили, по логике же дело в следующем.


У операции Max над int крайне неудобный нейтральный элемент — int.MinValue, он зависит от типа данных и очень редко имеет смысл в бизнес-логике. Проще исключение кинуть.


А вот над int? нейтральным элементом становится очень удобный null, имеющий смысл независимо от используемого числового типа данных, а также осмысленно используемый в бизнес-логике (не просто же так Nullable<int> на входе-то). Поэтому логично в ответ на пустую последовательность этот нейтральный элемент и вернуть.

Да, получается интересно — если бы не было метода с Nullable, то вызывался бы метод с обычным селектором, а так вызывается с Nullable.

Что до нейтрального элемента — сказать бы это прикладным программистам, большинство которых в моей практике использовало 0 (ноль), DateTime.MinValue и подобное как нейтральный элемент, или приравнивая его к нейтральному элементу Nullable(T).
Если бы не было метода с Nullable<int> — то была бы ошибка компиляции :-)

Что же до прикладных программистов…

Ноль как нейтральный элемент работает пока нет отрицательных чисел, немало олимпиадников на этом погорели :-)

Ну а DateTime.MinValue ужасно работает в базах данных — там у каждой СУБД свои диапазоны для дат, и DateTime.MinValue может либо оказаться невалидным значением, либо недостаточно минимальным. С ним одни проблемы…

Короче, все эти статьи возникни на неправильном использовании дотнета, плюс они помогают и дальше его неправильно использовать. Неправильно это.


p.s. у меня как раз почти весь код использует IReadOnlyCollection, IReadOnlyList (если доподнительно нужен доступ по индексу), есть в том числе и IReadOnlyDictionary.
Этот ReadOnly-набор интерфейсов такой офигенный, и логическая потребность в нем настолько очевидна, что я искал в дотнете нечто подобное еще когда она даже не появилась (с 4.0 версии ввели).


Очень странно, что сейчас (уже вышел 4.7/7.2, есть куча ресурсов, русский SO, всякие митапы, и т.д.) все еще есть люди, которые так косячат, а что еще более странно, что на хабрe появляются статьи, которые вместо того, чтобы указать им верный путь, указывают этим людям как им продолжить писать неправильный код.

Просто пример из жизни, надо получить из базы данных (удаленного сервиса, файла и т.д.) список сущностей имея IEnumerable их идентификаторов:
List<Entitiy> ReadEntities(IEnumearble<int> entityIds){…}

Если не проверять entityIds на пустоту, то может возникнуть ситуация, когда выполняется ненужный ресурсоемкий запрос.
И это это как раз пример неправильного API, о котором я пишу выше.
entityIds к моменту вызова ReadEntities должен быть простым списком/массивом/коллекцией в памяти.
Поэтому, чтобы принять аргумент любого этих этих типов, методу достаточно иметь сигнатуру:

List<Entity> ReadEntities(IReadOnlyCollection(int) entityIds){…}

И если возникнет необходимость проверить пустоту — есть свойство Count.

И еще, если уж у вас entityIds — IEnumerable(int) (т.е., в общем случае бесконечная и ленивая последовательность, вплоть до ленивого чтения из БД), то и результат метода должен быть IEnumerable(Entity) — иначе, если смотреть строго, размера List может не хватить, да и если у вас входной аргумент может быть ленивым, и результат то тем более должен быть ленивым:

IEnumerable<Entity> ReadEntities(IEnumerable(int) entityIds){…}

Так что изначальная сигнатура
List<Entitiy> ReadEntities(IEnumerable(int) entityIds)
— пример не только того, как List и IEnumerable используются в прикладном API не по назначению, но и еще сочетаются совершенно эклетичным образом.
Хочу немного защитить именно такой контракт:
List<Entitiy> ReadEntities(IEnumerable(int) entityIds)
IEnumerable(int) на входе, дает нам возможность без лишних буферов в памяти использовать LINQ
Пример “от балды”:
ReadEntities(items.Where(i=> i > 1000))

Что По поводу типа результата List<Entitiy>, то в случае чтения из базы данных я соглашусь что ленивое чтение было бы предпочтительнее (хотя есть нюансы, о которых чуть позже), но в случае с обычным REST Api “ленивое” чтение уже не выйдет. Теперь про нюансы с ленивым чтением — в наши дни, думаю, многие уже поняли важность неблокирующего кода, потому в реальности контракт выглядел бы так:
Task<List<Entitiy>> ReadEntities(IEnumerable(int) entityIds)

но тут ленивое чтение не особо то выходит, хотя можно было бы сделать неблокирующий код с ленивым чтением вот таким образом:
IObservable<Entitiy>> ReadEntities(IEnumerable(int) entityIds)

но все же мой идеал контракта это:
IAsyncEnumerable<Entitiy> ReadEntities(IEnumerable(int) entityIds)

Ждем C#8 :)
Хочу немного защитить именно такой контракт:
List(Entitiy) ReadEntities(IEnumerable(int) entityIds)

в случае с обычным REST Api “ленивое” чтение уже не выйдет

REST — ок, а почему именно List, а не хотя бы IList? (по аналогии с IEnumerable)
Или почему не массив, к примеру?
Откуда у .NET-разработчиков эта любовь к конкретной реализации List в публичном прикладном API?


А если еще точнее, почему не IReadOnlyList? Зачем возвращать изменяемые данные?
Если мы бы возвращали IEnumerable (или желаемый IAsyncEnumerable), то там коллекция была бы неизменяемой, и это правильно:


  • Мы сделали некоторую выборку по id (т.е., уже "удалили" из общего объема ненужные данные) и, по идее, должны возвратить некоторую неизменяемую со стороны клиентского кода коллекцию. Нужно поменять зачем-то — пусть клиент делает новую выборку из результата и получает новые данные. Т.е., "вот это все" — функциональщина, иммутабельность и прочее, к чему нас тот же IEnumerable подвигает. Зачем List?
  • Совсем в идеале я бы видел использование IImmutableList вместо IReadOnlyList, но первый из коробки есть только в .NET Core.
По поводу ReadOnly и IImmutableList полностью согласен, я их всегда использую в подобных методах, тут я просто не хотел отвлекать внимание и лишь указать на тот факт, что метод возвращает ссылку на список расположенный в памяти.
Откуда у .NET-разработчиков эта любовь к конкретной реализации List в публичном прикладном API?

Если метод возвращает List<T>, то по нему можно сделать foreach без выделения памяти под IEnumerator<T> (поскольку он является struct). При перечислении через интерфейс итератор будет упакован.
Понятно, что если есть хоть какая-то вероятность, что List<T> понадобится заменить, лучше не экономить на спичках и возвращать интерфейс. Видимо, сказывается тяга к преждевременной оптимизации.

Вряд ли разработчики, бездумно выставляющие реализацию List(T) в контрактах API, исходят из таких деталей.
Откуда у .NET-разработчиков эта любовь к конкретной реализации List в публичном прикладном API?

Насчет публичного — не скажу, но в случае внутреннего — все очень просто.


  1. Использование интерфейса не упрощает вызывающий код, а иногда может даже усложнить (да, оно упрощает архитектуру — но до понимания этого факта надо дорасти)
  2. Это самый легковесный из всех контейнеров
  3. У него есть метод ForEach
Это самый легковесный из всех контейнеров

T[] будет чуть-чуть полегковеснее. А ImmutableArray<T> вообще struct.
Так что List<T> явно не победитель

T[] в каком-то смысле легковеснее, но его еще надо создать как-то. Коллекции, все же, создаются обычно так: items.Add(nextItem)
T[] в каком-то смысле легковеснее, но его еще надо создать как-то.

.ToArray() из Linq, который работает через struct LargeArrayBuilder


Работает по смыслу примерно так же как и .ToList() (создаём небольшой контейнер, если не помещается, ресайзим в геометрической прогрессии), поэтому проблем с возвратом массивов из методов нет.

Знаете что самое занятное в данной ситуации. Вы заморачиваетесь про лишние буфферы памяти, пытаетесь предусмотреть какие-то оптимизации и прочее. А на практике в этом всём нет смысла, потому что вы не знаете как устроен ReadEntities. И постоянно делаете свои предположения (основанные лично на вашем опыте) о том, как он там что делает. А может он там внутри 50 раз бегает по всей переданной коллекции. И тогда очень плохо, что ему позволено на вход получить IEnumerable, а лучше бы это был IList. Или же разработчик этого метода наоборот знал, что да, придётся 50 раз в случайном порядке пробежаться по переданной коллекции, поэтому 1й же строчкой делает .ToList(); Или же ещё лучше — разработчик этого метода бежит по коллекции последовательно, всего 1 раз и заодно во время итерирования и проверил, а было ли ему передано хоть что-то.

А вы тут со своим «может возникнуть ситуация, когда выполняется ненужный ресурсоемкий запрос». Может. Как и ещё тысяча других, более ужасных.

Поэтому нужно не усложнять код (всевозможными условиями, обёртками, бла-бла), а наоборот — упрощать (более точным или гранулярным API, етц). Меньше кода, меньше абстракций — дешевле это счастье поддерживать и быстрее отправлять в продакшн.

Конкретно для суммы подойдет вот такой вариант:


int? result = GetItemsLinq()?.Sum();

Отличие от исходного варианта — для перечисления из 0 элементов будет не null, а 0 — но это математически корректно.


Что же до опасностей Any — лично мне достаточно просто знать что я делаю.

Представьте себе последовательность 1,-1, 2,-2… сумма тоже будет 0, но ситуация это совсем другая.

Да, вот мой вариант для того чтобы избегать двойного перечисления:


    class Buffer<T> : IDisposable, IEnumerable<T>
    {
        private readonly List<T> buf = new List<T>();
        private readonly IEnumerator<T> enumerator;
        private readonly IReadOnlyCollection<T> collection;

        public Buffer(IEnumerable<T> enumerable)
        {
            this.collection = enumerable as IReadOnlyCollection<T>;
            if (collection == null)
            {
                this.enumerator = enumerable.GetEnumerator();
            }
        }

        public void Dispose() => enumerator?.Dispose();

        public IEnumerator<T> GetEnumerator() => collection == null
            ? new Enumerator(buf, enumerator)
            : collection.GetEnumerator();

        IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();

        class Enumerator : IEnumerator<T>
        {
            private readonly List<T> buf;
            private readonly IEnumerator<T> enumerator;
            private int i = 0;

            public Enumerator(List<T> buf, IEnumerator<T> enumerator)
            {
                this.buf = buf;
                this.enumerator = enumerator;
            }

            public T Current => buf[i];
            object IEnumerator.Current => Current;

            public void Dispose() { }

            public bool MoveNext()
            {
                if (++i < buf.Count) return true;
                if (!enumerator.MoveNext()) return false;

                buf.Add(enumerator.Current);
                return true;
            }

            public void Reset()
            {
                i = 0;
            }
        }
    }

Это будет работать в любом случае, не только для пары Any() и Sum()


Да, это тоже промежуточный буфер — но обычно он все-таки нужен...

Мне кажется, что в метод суммирования не надо передавать null. По хорошему, это клиентский код должен проверять на null IEnumerable. В вашем примере ему же всё равно результат нужно будет проверить на null, так почему бы не проверить параметр и не обеспечить early return?
Если проверку внутри метода суммирования убрать, то необходимость в extension method отпадает совсем.

Но если оставить всё как есть, всё равно не понимаю, зачем эта пляска с инумератором, прямо все кишки Enumerable наружу, бррр. Можно же проще и понятнее, чтобы даже тупой разобрался, что этот код делает:

private static int? GetSum(IEnumerable<int> items)
{
	if (items == null) return null;

	bool any = false;
	int sum = 0;
	foreach (int item in items)
	{
		any = true;
		sum += item;
	}
	if (any) return sum;
	return null;
}
По поводу проверки на null, статья все же была не об этом. Проверять или не проверять на нулл это тема отельного большого холивара. По моему мнению, возможность передачи аргументов со значением null должна быть как-то оговорена в контракте метода, но к сожалению, пока в C# нет встроенных возможностей указать необходимость наличия значения у аргументов с ссылочными типами – ждем C#8. Сейчас можно использовать атрибуты [Null] [NotNull] и какой-нибудь статический анализатор и в любом случае ситуация с null должна быть оговорена в документации к методу.
По поводу вашей реализации GetSum – она вполне может применяться, но она не менее громоздкая. К тому же у вас введены две изменяемые переменные, что совершенно не критично, но идет в разрез с современными тенденциями в программировании (это я про ФП)
Эти две изменяемые локальные переменные ничуть не мешают функции GetSum быть чистой (при условии что перечисление items не имеет побочных эффектов).

Но можно и аналогичный код с использованием Aggregate слепить если функциональный стиль — самоцель.
ФП не самоцель, и я согласен что GetSum остается «чистой».

"Дело в том, что для items.Any() необходимо перебрать 91 оригинальный элемент"


Разве это верно? Имхо Any достаточно чтобы MoveNext 1 раз сработал.
Вот Count как раз начнет перебирать все чтобы узнать количество.

Имхо Any достаточно чтобы MoveNext 1 раз сработал

да, но только для последнего итератора, в котором что бы получить первый исходящий элемент (<10) нужно перебрать 91 входящий элемент (>= 10).
Sign up to leave a comment.

Articles