Возвращая коллекцию в метод интерфейса IEnumerable


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

Зная, что PricingInformation это ListЭто мое возвращение заявления считают лучшей практикой или нет? Я должен отложить казнь на более поздний срок? Или это разумно ToArray() в Select во время возвращения заявления?

Какой из этих подходов вы думаете, что больше подойдет?

public IEnumerable<ProductPrice> GetPrices(IEnumerable<string> skus, string accountNumber)
{
    if (skus.Any())
    {
        TesscoModel.PricingQuery priceQuery = new TesscoModel.PricingQuery();
        foreach (string sku in skus)
            priceQuery.Skus.Add(new TesscoModel.SkuDetail() { Sku = sku, Quantity = 1 });
        priceQuery.Account = accountNumber;
        TesscoModel.PricingInformationBatch pricingInfoBatch = RestHelper.PostJsonAsync<TesscoModel.PricingInformationBatch>(Settings.PricingServiceEndpoint, priceQuery).Result;

        return pricingInfoBatch?.PricingInformation?.Select(pi => new ProductPrice { Sku = pi.Sku, Price = pi.OnSalePrice != default(decimal) ? pi.OnSalePrice : pi.UnitPrice, ListPrice = pi.ListPrice })?.ToArray() ?? Enumerable.Empty<ProductPrice>();
    }
    else
    {
        return Enumerable.Empty<ProductPrice>();
    }
}

Этот метод затем вызывается другой метод внутри helper. Внутри которого я использую FirstOrDefault чтобы найти первый sku. FirstOrDefault снова enumerator.

Я думаю, что отличалось исполнение более уместно здесь, потому что я не должен исполнять указатель, пока мне не придется. Я думаю, что с помощью ToArray() слишком рано, я держу объект на памяти слишком долго. Хотя, это может не потребоваться.

//Get sku Pricing in bulk
IEnumerable<ProductPrice> productPrices = productService.GetPrices(skus, (!account.IsNull()) ? account.AccountNumber : string.Empty);

foreach (var line in lines)
{
    var productPrice = productPrices.FirstOrDefault(p => p.Sku.Equals(line.Sku));
    line.ListPrice = productPrice?.ListPrice ?? default(decimal);
    line.YourPrice = productPrice?.Price ?? default(decimal);
}


153
2
задан 14 апреля 2018 в 06:04 Источник Поделиться
Комментарии
2 ответа

Нет 0 и 1 ответ


X любит выполнять IEnumerable как можно скорее, поэтому Х делает его внутри метода.

А


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

Они оба не правы. Ответ не Цифровой, это, нет, 0 или 1 ответ. Это зависит от того. Иногда вы должны выполнить сбор и возврат неIEnumerable.

Материализуются при работе с базами данных

Таком случае будет выполнять запросы к базе данных. В этом случае вы бы открыть подключение к базе данных (или создать контекст, если это какой-ОРМ) и распоряжаться его в конце. Если вы не выполните его ToList или ToArray и т. д. вы не сможете сделать это позже. Выполнения обернуть перечислитель с анонимный тип, содержащий контекст, но когда исполнение оставляет метод, контекст получает распоряжался и бросить excepiton данный момент Вы пытаетесь получить данные. В этой ситуации вы должны вернуть неIEnumerable тип.

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

Итак, в сухом остатке: если у вас нет материализовать результат (потому что вы не сможете сделать это позже, когда служба/поставщик утилизировать и т. д.), Не делайте этого, но будьте последовательны с API (возвращаемые типы).


Когда в результате это овеществленный я жду вас донести этот факт наглядно с помощью ICollection, IList или что-нибудь еще, но IEnumerable.


Материализовать для упрощения отладки при вызове ресурсов

Что касается вашего кода вызове веб-сервиса есть


RestHelper.PostJsonAsync(..)

Так что вы должны спросить себя: Является ли это ОК, чтобы быть сделано несколько раз, если кто-то еще использует Any и потом начинается перечисление его снова или лучше не допускать таких ситуаций (по любой причине)?

Я хотел бы использовать IList здесь одна причина: это очень трудно отлаживать ленивый код, который вызывает веб-сервисы, базы данных или другие ресурсы. Когда что-то идет не так, вы будете иметь большое удовольствие.


Считать материализованной параметров

Те же принципы применяются к параметрам. В вашем примере вы выполняете skus несколько раз: с Any и с foreach петли.

Здесь, я бы просил ICollection или IList попросил звонящего: Берегись! Я буду использовать свою коллекцию более одного раза (так что вы могли бы рассмотреть материализуя его для повышения производительности до гивит мне).

1
ответ дан 15 апреля 2018 в 05:04 Источник Поделиться

Читабельность

Вы склонны зубрить много кода в несколько строк. Некоторые линии просто слишком длинный и сложный, на мой взгляд. Есть несколько вещей, которые я бы сделал по-другому:


  • var позволяет удалить дубликаты типа "имя экземпляра".

  • "" является более емким, чем string.Empty. То же самое касается 0m против default(decimal). Лично мне гораздо больше нравится "" и 0mне только потому, что они короче, но и потому, что их фактическая стоимость сразу узнаваем.

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

  • Для цепочечной (метод в LINQ) звонки, вы можете положить каждый вызов метода на отдельной строке.

  • Троичная МФС не очень читабельно, особенно когда та же линия также содержит как Элвис и нуль-коалесцирующий оператора. Еще одна причина, чтобы разделить вещи на несколько строк.

  • Многие из участвующих классов отсутствуют полезные конструкторы. Не только и делает, что требует больше времени, чтобы писать, он также позволяет легко создавать недопустимые объекты (я предполагаю, что SkuDetail объект без Sku не действует, например?). Если вам нравится выписывать имена свойств для ясности, это также возможно с именами аргументов.

  • Зачем ты пишешь TesscoModel пространство имен (или внешнего класса?) префикс везде?

Другие ноты


  • Вы можете представить себе скорейшее возвращение путем изменения if (skus.Any()) в if (!skus.Any()), которые должны сделать остальные способ немного легче читать (меньше отступ уровней, ни края-дело регулировать потом).

  • Вы уверены GetPrices не должно быть async способ? Извлечение Result как это будет блокировать текущий поток, пока результат налицо.

  • Используя default(decimal) (или просто 0) указывая 'не имеет значения' выглядит довольно хрупкой для меня. Это легко забыть, проверить, как, что, и это может вызвать продукты, которые должны быть отданы бесплатно (Цена = 0)... с помощью decimal? вместо этого позволяет четко показать, что там не может быть значение, и это заставляет вызывающий код, чтобы проверить это.

Отложенное выполнение

Что касается вашего вопроса, в данном конкретном случае отложенного выполнения на самом деле имеет негативные последствия. Каждый раз, когда вы перебираете результат (который FirstOrDefault делает) Select должен повторить - не только на первый совпадающий элемент, но и для всех предыдущих, а также. Эта работа должна быть повторена для каждого следующего FirstOrDefault звоните, и поскольку результаты не кэшируются в любом месте, что, вероятно, означает, что много работы будет впустую.

Например, если у одного SKU на линию, то вместо n вы в конечном итоге с (n^2 + n) / 2 ProductPrice объекты создаются. Это квадратичная, а не линейная!

Тут же материализуя результаты не страдают от этой проблемы. Но если вы ожидаете много наименований, то это, вероятно, лучше создать словарь, который сопоставляет SKUs на ProductPrice объекты. Это займет больше работы, чем создание массива, но поиск займет постоянной, а не линейное время, так что вы в конечном итоге с более масштабируемое решение.

1
ответ дан 15 апреля 2018 в 06:04 Источник Поделиться