3

Есть коллекция Dictionary<string, MyClass> products = new Dictionary<string, MyClass>(); Одно из свойств класса MyClass - Group (группа товара), тип string. Мне нужно узнать есть хоть один элемент в коллекции, у которого свойство Group = "Мясо", к примеру.

if (products.Where(x=>x.Value?.Group == "Мясо").Count()>0)

Так корректно? Спасибо. Мне важно, чтобы максимально быстро была проверка

14
  • 1
    забыли ToList, а так все должно работать.
    – Monomax
    12 апр 2019 в 8:00
  • 6
    Вместо .Where(x=>x.Value?.Group == "Мясо").Count()>0 проще написать Any(x=>x.Value?.Group == "Мясо")
    – Bulson
    12 апр 2019 в 8:02
  • 2
    В любом случае linq будет уступать в производительности обычному перебору с помощью цикла. К тому же у вас запрос будет осуществлять проход от первого до последнего элемента, хотя правильнее было бы останавливаться, когда хотя бы один найден. 12 апр 2019 в 8:02
  • 3
    @AndreiKhotko, не в любом.
    – Grundy
    12 апр 2019 в 8:10
  • 2
    @Grundy можете привести пример, пожалуйста? 12 апр 2019 в 8:38

1 ответ 1

5

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

Ваш запрос неоптимален. У вас сначала отбираются элементы через Where, потом считается их число. И только потом проверяется на ноль.

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

Вы же так не стали бы делать, верно? Вот и мы давайте не будем делать.

Можно объединить Where и Count:

if (products.Count(x=>x.Value?.Group == "Мясо")>0)

Уже стало лучше. Однако стоит ли перебирать весь массив или достаточно найти хотя бы один элемент и как только нашли - сразу выйти из цикла?

[__DynamicallyInvokable]
public static bool Any<TSource>(this IEnumerable<TSource> source, Func<TSource, bool> predicate)
{
  if (source == null)
    throw Error.ArgumentNull(nameof (source));
  if (predicate == null)
    throw Error.ArgumentNull(nameof (predicate));
  foreach (TSource source1 in source)
  {
    if (predicate(source1))
      return true;
  }
  return false;
}

Заменяем Count на Any:

if (products.Any(x=>x.Value?.Group == "Мясо"))

Всё, в пределах linq больше оптимизировать некуда.

Отдельное замечание нужно сделать по поводу того, что могла бы первая оптимизация сделана самим компилятором при условии, что у вас был бы IQueryable, а не IEnumerable, но я не уверен, что компилятор бы смог заменить Count > 0 на Any - он бы честно посчитал.

Стоит ли заменять linq на цикл? Не стоит - я показал, что под капотом у Any тот же самый цикл. А выглядит такая linq конструкция короче и семантичнее, а потому понятнее при чтении.

3
  • 1
    У вас сначала отбираются элементы через Where, потом считается их число. ну не, это ж не так работает тут же итераторы
    – Grundy
    12 апр 2019 в 8:37
  • 1
    @Grundy Это правда, но есть два "но", оба учебно-педагогического характера. Во-первых, задающие подобного рода вопросы не держат решарпер, который предложил бы упростить код. Поэтому когда-нибудь доберутся и увидят эти рефакторинги - поймут о чём была речь. Во-вторых, мне так нагляднее пояснить мысль про два цикла и показать на этом наглядном примере ненужность.
    – A K
    12 апр 2019 в 8:41
  • 1
    Но фактически проход по массиву будет один
    – Grundy
    12 апр 2019 в 8:45

Ваш ответ

By clicking “Отправить ответ”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Всё ещё ищете ответ? Посмотрите другие вопросы с метками или задайте свой вопрос.