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

Question

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

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

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

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

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

Для меня это похоже два набора 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. Кроме того, вспоминая, что мои старые учителя проповедуют: «для каждого цикла существует лучшая структура»

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

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

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

Решение

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

к сожалению, я не мог заставить его работать с любыми запросами 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 
    } 
} 

Answer 1

Сначала вы можете 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. 
    } 
} 

Answer 2

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

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

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

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