Оптимизация алгоритма и / или структуры в C # - PullRequest
9 голосов
/ 08 марта 2012

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

При отправке электронных писем (что является запланированной дегустацией), я должен посмотреть, на какие города и на какие категории подписчик подписался, перед отправкой электронного письма. То есть, если я выбрал «Лондон» и «Манчестер» в качестве городов по своему выбору и выбрал «Продукты питания», «Ткань» и «Электроника» в качестве своих категорий, я получу только те информационные бюллетени, которые относятся к ним.

Структура выглядит следующим образом:

На каждом новостном элементе в Umbraco CMS есть разделенная запятыми строка городов и категорий (по сути, они хранятся в виде идентификаторов узлов, поскольку города и категории также являются узлами в Umbraco) Когда я подписываюсь на один или несколько городов и одну или несколько категорий Я храню ноды города и категории в базе данных в пользовательских таблицах. Мое реляционное отображение выглядит так:

enter image description here

И вся структура выглядит так:

enter image description here

Мне кажется, что это два набора отношений 1 - 1 .. * (один подписчик на один или несколько городов и один подписчик на одну или несколько категорий)

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

private bool shouldBeAdded = false;

// Dictionary containing the subscribers e-mail address and a list of news nodes which should be sent
Dictionary<string, List<Node>> result = new Dictionary<string, List<Node>>();

foreach(var subscriber in subscribers)
{
    // List of Umbraco CMS nodes to store which nodes the html should come from
    List<Node> nodesToSend = new List<Node> nodesToSend();

    // Loop through the news
    foreach(var newsItem in news)
    {
        // The news item has a comma-separated string of related cities
        foreach (string cityNodeId in newsItem.GetProperty("cities").Value.Split(','))
        {
            // Check if the subscriber has subscribed to the city
            if(subscriber.CityNodeIds.Contains(Convert.ToInt32(cityNodeId)))
            {
                 shouldBeAdded = true;
            }
        }

        // The news item has a comma-separated string of related categories
        foreach (string categoryNodeId in newsItem.GetProperty("categories").Value.Split(','))
        {
            // Check if the subscriber has subscribed to the category
            if(subscriber.CategoryNodeIds.Contains(Convert.ToInt32(categoryNodeId)))
            {
                shouldBeAdded = true;
            }
        }
    }

    // Store in list
    if (shouldBeAdded)
    {
        nodesToSend.Add(newsItem);
    }

    // Add it to the dictionary
    if (nodesToSend.Count > 0)
    {
        result.Add(subscriber.Email, nodesToSend);
    }
}

// Ensure that we process the request only if there are any subscribers to send mails to
if (result.Count > 0)
{
    foreach (var res in result)
    {
        // Finally, create/merge the markup for the newsletter and send it as an email.
    } 
}

Хотя это работает, меня немного беспокоит производительность, когда достигается определенное количество подписчиков, так как мы находимся в трех вложенных циклах foreach. Кроме того, вспоминая мои старые учителя, проповедует: «для каждого цикла for есть лучшая структура»

Итак, я хотел бы, чтобы вы высказали свое мнение по поводу вышеуказанного решения, есть ли что-то, что можно улучшить здесь с данной структурой? И не вызовет ли это со временем проблем с производительностью?

Любая помощь / подсказка очень ценится! : -)

Заранее спасибо.

Решение

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

К сожалению, я не смог заставить его работать с любыми запросами LINQ, которые я пробовал, поэтому я вернулся к «старому» способу итерации ;-) Окончательный алгоритм выглядит так:

private bool shouldBeAdded = false;

// Dictionary containing the subscribers e-mail address and a list of news nodes which should be sent
Dictionary<string, List<Node>> result = new Dictionary<string, List<Node>>();

foreach(var subscriber in subscribers)
{
    // List of Umbraco CMS nodes to store which nodes the html should come from
    List<Node> nodesToSend = new List<Node> nodesToSend();

    // Loop through the news
    foreach(var newsItem in news)
    {
         foreach (string cityNodeId in newsItem.GetProperty("cities").Value.Split(','))
         {
             // Check if the subscriber has subscribed to the city
             if (subscriber.CityNodeIds.Contains(Convert.ToInt32(cityNodeId)))
             {
                 // If a city matches, we have a base case
                 nodesToSend.Add(newsItem);
             }
         }

         foreach (string categoryNodeId in newsItem.GetProperty("categories").Value.Split(','))
         {
             // Check if the subscriber has subscribed to the category
             if (subscriber.CategoryNodeIds.Contains(Convert.ToInt32(categoryNodeId)))
             {
                 shouldBeAdded = true;

                 // News item matched and will be sent. Stop the loop.
                 break;
             }
             else
             {
                 shouldBeAdded = false;
             }
         }

         if (!shouldBeAdded)
         {
             // The news item did not match both a city and a category and should not be sent
             nodesToSend.Remove(newsItem);
         }
    }

    if (nodesToSend.Count > 0)
    {
        result.Add(subscriber.Email, nodesToSend);
    }  
}

// Ensure that we process the request only if there are any subscribers to send mails to
if (result.Count > 0)
{
    foreach (var res in result)
    { 
        // StringBuilder to build markup for newsletter
        StringBuilder sb = new StringBuilder();

        // Build markup
        foreach (var newsItem in res.Value)
        {
            // build the markup here
        }

        // Email logic here
    }
}

Ответы [ 2 ]

2 голосов
/ 08 марта 2012

Во-первых, вы можете break свой внутренний foreach, как только shouldBeAdde = true.

Вы также могли бы использовать LINQ, но я не уверен, будет ли он быстрее (но вы могли бы использовать .AsParallel, чтобы сделать его многопоточным легко):

var nodesToSend = from n in news
                 where n.GetProperties("cities").Value.Split(',')
                     .Any(c => subscriber.CityNodeIds.Contains(Convert.ToInt32(c)) &&
                 n.GetProperties("categories").Value.Split(',')
                     .Any(c => subscriber.CategoryNodeIds.Contains(Convert.ToInt32(c))
                 select n;

Полное мнение будетзатем снизитесь до (включая параллель):

Dictionary<string, IEnumerable<Node>> result = new Dictionary<string, IEnumerable<Node>>();
foreach(var subscriber in subscribers)
{
    var nodesToSend = from n in news.AsParallel()
        where n.GetProperties("cities").Value.Split(',')
                .Any(c => subscriber.CityNodeIds.Contains(Convert.ToInt32(c)) &&
            n.GetProperties("categories").Value.Split(',')
                .Any(c => subscriber.CategoryNodeIds.Contains(Convert.ToInt32(c))
        select n;

    if (nodesToSend.Count > 0)
        result.Add(subscriber.Email, nodesToSend);
}

if (result.Count > 0)
{
    foreach (var res in result)
    {
        // Finally, create/merge the markup for the newsletter and send it as an email.
    } 
}
0 голосов
/ 08 марта 2012

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

Сказав это, возможной оптимизацией может быть следующее:

Вы можете сохранить отношение между городом и подписчиком в словаре с ключом города и подписчиками для этого города в качестве значения словаря, хранящегося как HashSet<T>. И вы можете сделать то же самое для категории для подписчика.

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

...