Tối ưu hóa một thuật toán và/hoặc cấu trúc trong C#

Question

Tôi đang làm việc trên một ứng dụng mà bạn có thể đăng ký nhận bản tin và chọn danh mục bạn muốn đăng ký. Có hai nhóm danh mục khác nhau: thành phố và danh mục.

Khi gửi email (là một thị hiếu theo lịch biểu), tôi phải xem thành phố nào và danh mục nào mà người đăng ký đã đăng ký trước khi gửi email. Tức là, nếu tôi đã đăng ký "London" và "Manchester" làm thành phố được lựa chọn và đã chọn "Thực phẩm", "Vải" và "Điện tử" làm danh mục của tôi, tôi sẽ chỉ nhận được các bản tin liên quan đến chúng.

Cấu trúc như sau:

Trên mỗi newsitem trong Umbraco CMS có một chuỗi commaseparated của các thành phố và các loại (có hiệu quả, đây là những lưu trữ như id nút từ các thành phố và loại là các nút trong Umbraco là tốt) Khi tôi đăng ký một hoặc nhiều thành phố và một hoặc nhiều danh mục, tôi lưu trữ các nút thành phố và danh mục trong cơ sở dữ liệu trong các bảng tùy chỉnh. bản đồ quan hệ của tôi trông như thế này:

Và toàn bộ cấu trúc trông như thế này:

Đối với tôi, điều này có vẻ như hai bộ 1-1 .. * quan hệ (một người đăng ký một hoặc nhiều thành phố và một người đăng ký một hoặc nhiều danh mục)

Để tìm email nào gửi người đăng ký, mã của tôi trông giống như sau:

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

Trong khi công việc này, tôi hơi lo ngại về hiệu suất khi đạt được số lượng người đăng ký nhất định kể từ khi chúng tôi tham gia ba vòng lặp forested lồng nhau. Ngoài ra, hãy nhớ rằng các giáo viên cũ của tôi rao giảng: "cho mỗi vòng lặp có một cấu trúc tốt hơn"

Vì vậy, tôi muốn phản đối của bạn về giải pháp trên, có điều gì có thể được cải thiện ở đây không? Và nó sẽ gây ra vấn đề hiệu suất theo thời gian?

Bất kỳ trợ giúp/gợi ý nào được đánh giá cao! :-)

Xin cảm ơn trước.

Giải pháp

Vì vậy, sau một vài giờ tốt gỡ lỗi và fumblin' xung quanh cuối cùng tôi đã đưa ra một cái gì đó mà làm việc (ban đầu, nó trông như mã ban đầu của tôi làm việc, nhưng nó đã không)

Đáng buồn thay, tôi không thể có được nó để làm việc với bất kỳ truy vấn LINQ tôi đã cố gắng, vì vậy tôi quay trở lại "ol 'trường phái' cách lặp ;-) thuật toán cuối cùng trông như thế này:

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

Trước tiên, bạn có thể break bạn tham gia nội bộ ngay sau shouldBeAdde = true.

Bạn cũng có thể sử dụng LINQ, nhưng tôi không chắc chắn nếu nó sẽ nhanh hơn (nhưng bạn có thể sử dụng .AsParallel để làm cho nó dễ dàng multithreaded):

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; 

Những suy nghĩ hoàn toàn sau đó sẽ đi xuống (incl. parallel):

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

Tôi không nghĩ bạn sẽ sẽ sớm gặp phải sự cố về hiệu suất. Tôi sẽ để nó giống như bạn có nó ngay bây giờ và chỉ cố gắng tối ưu hóa sau khi bạn gặp phải một vấn đề hiệu năng thực sự và đã sử dụng một profiler để xác minh rằng các vòng này là vấn đề. Hiện tại, có vẻ như bạn đang thực hiện tối ưu hóa sớm.

Có nói rằng, sau đây có thể là một tối ưu hóa khả năng:

Bạn có thể lưu trữ các mối quan hệ từ thành phố đến thuê bao trong một cuốn từ điển với các thành phố như là chìa khóa và các thuê bao cho thành phố này như giá trị của từ điển được lưu trữ dưới dạng HashSet<T>. Và bạn có thể làm tương tự cho danh mục cho người đăng ký.

Bây giờ khi bạn gửi bản tin, bạn có thể lặp lại các mục tin tức bạn có thể truy xuất thuê bao cho các thành phố bằng từ điển và bạn có thể truy xuất thuê bao cho các danh mục bằng từ điển. Bây giờ bạn cần phải giao cắt các thuê bao thành phố HashSet với các thuê bao loại HashSet và kết quả là bạn sẽ có tất cả các thuê bao phù hợp cho mục tin tức.