1

I have some code in C#, I am not very good in C# so used loops inside loops but they are taking too long. Is there any way to write code for very fast execution to save time with accuracy?

Here is the code.

    foreach (var vmain in vendorMainResult)
    {
        foreach (var povendor in potoDateOrders)
        {
            if (vmain.VendorNumber == povendor.VendorNumber && vmain.Year == povendor.Year)
            {
                vmain.ToDateOrders = povendor.ToDateOrders;
                vmain.OutstandingComm = povendor.OutstandingComm;
                break;
            }
        }
    }

Think about if there are 20,000 records in each collection then 20k x 20k takes 1-2 minute.

Builder
  • 1,046
  • 2
  • 10
  • 30
  • If your potoDateOrders list was ordered on vendor number and year, you wouldn't have to iterate over so many times. – snow_FFFFFF Oct 15 '15 at 15:40
  • According to the [documentation](https://msdn.microsoft.com/en-us/library/dd460720%28v=vs.110%29.aspx), it is possible to run foreach on multiple thread. It can give you better perf on your operations. – Xavier W. Oct 15 '15 at 15:42
  • 2
    @XavierWOLFF Rather than doing lots of superfluous work in parallel, it's way faster to just not do lots of superfluous work. – Servy Oct 15 '15 at 15:44
  • I would try and [get a free profiler](http://stackoverflow.com/questions/3927/what-are-some-good-net-profilers) and actually measure where all the time is being taken up. If you don't profile you only have "guesses" on what is slow, if you do profile you will "know" what is slow. – Scott Chamberlain Oct 15 '15 at 15:46
  • @Servy Of course you are right. But we doesn't have enough informations about the context. So if he can't avoid superfluous work, it would at least be a little better to use Parralel class. – Xavier W. Oct 15 '15 at 15:46
  • Consider other data structures (rather than a list/array) that could give you faster lookup. For example, [`Dictionary<,>`](https://msdn.microsoft.com/en-us/library/xfhwa508%28v=vs.110%29.aspx) and [`HashSet<>`](https://msdn.microsoft.com/en-us/library/bb359438%28v=vs.110%29.aspx). (There are various ways to use these in your scenario--it's hard to pick one and write an answer because I don't know much about your classes and variables.) – 31eee384 Oct 15 '15 at 15:47
  • @ScottChamberlain He's implementing an inner join using a cross join and then filtering. Meaning it's implementing an O(N) algorithm as an O(N^2) algorithm. A profiler isn't really going to tell you that. He already did narrow the problem down to this section, which is about all the profiler is good for. – Servy Oct 15 '15 at 15:50
  • @XavierWOLFF We have more than enough information about the context, and there's *lots* of superfluous work to avoid. Using an improper and highly inefficient algorithm in parallel, rather than just using an efficient algorithm in the first place, is not the where you should be looking first. – Servy Oct 15 '15 at 15:52
  • You are making much more compares than the ones that are needed that's why your algorithm takes so much time O(vendorMainResult.Count * potoDateOrders.Count) – Christo S. Christov Oct 15 '15 at 15:53
  • @31eee384 There's exactly enough information in the question to refactor the given code from using loops into using a hash-based lookup structure. – Servy Oct 15 '15 at 15:57
  • @Servy I thought that at first but I realized that there aren't types for any of the properties and no hint about how the data is stored. A solution could be written that uses new example data but I don't know how useful that would be. – 31eee384 Oct 15 '15 at 16:02

1 Answers1

6

Use a Join to join two collections together efficiently.

var query = from vmain in vendorMainResult
    join povender in potoDateOrders
    on new 
    {
        vmain.VendorNumber, 
        vmain.Year,
    }
    equals new 
    {
        povendor.VendorNumber, 
        povendor.Year,
    }
    select new 
    {
        vmain,
        povendor,
    };
Servy
  • 202,030
  • 26
  • 332
  • 449
  • 2
    Though this is of course a correct way to solve the problem, I think it's a good idea to note that the win comes with some cost. The original looped algorithm was n-squared in time, yes, but it was constant in space. This algorithm is much faster in time, but imposes a much higher burden to build the join and to allocate temporary objects. Trading off more space for less time is usually a good idea, and it is a good idea in this case, but I think we want to recognize that the tradeoff is in fact being made. – Eric Lippert Oct 15 '15 at 16:01
  • I have added a left join to complete my solution but uses this solution to start my foundation. Comparing to loops, this is much faster. I had four loops to fill in main object which were taking 2.30 minutes. After changing all in this shape, now it is taking less than 4 seconds. Thanks @Sevy and all to help. – Builder Oct 15 '15 at 18:58
  • Can you help me on this question:http://stackoverflow.com/questions/35332928/better-or-optimized-way-to-filter-customer-records-by-3-dropdown-filters – I Love Stackoverflow Feb 12 '16 at 03:32