Optimizar un algoritmo y/o estructura en C#

Question

Estoy trabajando en una aplicación en la que puede suscribirse a un boletín informativo y elegir a qué categorías desea suscribirse. Hay dos grupos diferentes de categorías: ciudades y categorías.

Al enviar mensajes de correo electrónico (que es un tast programado), tengo que ver qué ciudades ya qué categorías se ha suscrito un suscriptor, antes de enviar el correo electrónico. Es decir, si me he suscrito a "Londres" y "Manchester" como mis ciudades de elección y he elegido "Comida", "Paño" y "Electrónica" como mis categorías, solo recibiré los boletines informativos relacionados con estos.

La estructura es la siguiente:

En cada elemento Novedades en Umbraco CMS no es una cadena commaseparated de las ciudades y categorías (efectivamente, éstos se almacenan como identificadores de nodo desde ciudades y categorías son nodos en Umbraco aswell) Cuando suscribirse a una o más ciudades y una o más categorías, almaceno los nodos de ciudad y categoría en la base de datos en tablas personalizadas. Mi mapeo relacional se ve así:

y toda la estructura se parece a esto:

Para mí, esto parece como dos juegos de 1 - 1 .. * relaciones (un suscriptor a una o varias ciudades y un suscriptor a una o más categorías)

Para encontrar qué correos electrónicos enviar a qué suscriptor, mi código se ve así:

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. 
    } 
} 

Mientras esto funciona, estoy un poco preocupado por el rendimiento cuando se llega a una cierta cantidad de suscriptores ya que estamos en tres bucles foreach anidados. Además, recordando que mis viejos profesores predican: "para cada ciclo forzado hay una mejor estructura"

Entonces, me gustaría su opinión sobre la solución anterior, ¿hay algo que pueda mejorarse aquí con la estructura dada? ¿Y causará problemas de rendimiento con el tiempo?

¡Cualquier ayuda/sugerencia es muy apreciada! :-)

Gracias de antemano.

Solución

Así que después de un par de buenas horas de depuración y fumblin' en torno finalmente se me ocurrió algo que funciona (en un principio, parecía que mi código original funcionó, pero no lo hicieron)

Lamentablemente, no pude conseguir que funcione con cualquier consultas LINQ que probé, así que volvió a la forma "ol 'escuela' de la iteración ;-) el algoritmo final se parece a esto:

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

Primero puede break usted foreach interno tan pronto como shouldBeAdde = true.

También es posible usar LINQ, pero no estoy seguro de si va a ser más rápido (pero se puede usar .AsParallel para que sea fácilmente multiproceso):

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; 

La reflexión completa sería luego bajar a (incluido el paralelo):

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

No creo que lo harás se encuentra con problemas de rendimiento pronto. Lo dejaría como si lo tuviera ahora y solo tratara de optimizarlo después de que se encuentre con un problema de rendimiento real y haya usado un generador de perfiles para verificar que estos bucles son el problema. Actualmente, parece que estás haciendo una optimización prematura.

Una vez dicho esto, el siguiente podría ser una posible optimización:

Se puede almacenar la relación de la ciudad al suscriptor en un diccionario con la ciudad como la clave y los suscriptores de esta ciudad como el valor del diccionario almacenado como HashSet<T>. Y puede hacer lo mismo para la categoría de suscriptor.

Ahora cuando envía su boletín informativo puede repetir las noticias para recuperar los suscriptores de las ciudades que utilizan el diccionario y puede recuperar los suscriptores de las categorías que utilizan el diccionario. Ahora debe cruzarse con los suscriptores de la ciudad HashSet con los suscriptores de la categoría HashSet y, como resultado, tendrá todos los suscriptores coincidentes para la noticia.