Foreach performs reduntant logic

Question

My two ForEach loops allow access to errorOrders (the user's name and their number of errors) and totalOrders (the user's name and their number of total orders).

My code constantly loops through these two ForEaches. The 'count' on both errorOrders and totalOrders is 38 and the program loops through all 38 users just fine. But then it keeps looping through them again and again, re-doing the process it just finished.

How can I loop through the users just once?

foreach (KeyValuePair<string, int> error in errorOrders)
{
    foreach (KeyValuePair<string, int> total in totalOrders)
    {

        errPercentage = ((double)error.Value / (double)total.Value);                        
        Console.WriteLine("Percentage of errors for " + total.Key + ": " + Math.Round(errPercentage, 2) * 100 + "%");
        ordersPerHour = OrdersPerHour(total.Key);
        RandomOrders = RandomSelect(errPercentage, total.Key);

        Console.WriteLine("Number of orders pulled : " + RandomOrders.Rows.Count);
        //Print out orders randomly collected
        for (int i = 0; i < RandomOrders.Rows.Count; i++) 
        {
            Console.WriteLine(RandomOrders.Rows[i]["ControlNumber"]);
        }

        Console.WriteLine("\r\n");
        //NumOrdersToPull = FindNumOrdersToPull(Math.Round(errPercentage,2), ordersPerHour);
    }

}

Answer 1

Separate the loops instead of nesting them. Putting one inside of the other makes the entire child loop run for each instance of the parent loop. (Hence the name.)

// (shared variables here)

foreach (KeyValuePair<string, int> total in totalOrders)
{
    // Code relevant to all orders here
}

foreach (KeyValuePair<string, int> error in errorOrders)
{
    // Code relevant to erroneous orders only here
}

If the loops need to share variables, declare these before the first loop. Variables created inside a loop will be local and stop existing when their loop is finished.

Answer 2

The second loop will run through once for every total in total orders. If you want them to only go through once each do not nest them. instead have them one after the other.

foreach(total in total orders)
{

}
foreach(error in errororders)
{

}

You would only need to nest them if they were related and you needed to do the inner loop once for every iteration of the outer loop.

Answer 3

Try this:

foreach (KeyValuePair<string, int> error in errorOrders)
{
    if (totalOrder.HasKey(error.Key) {
        var total = totalOrders[error.Key];

        errPercentage = ((double)error.Value / (double)total);                        
        Console.WriteLine("Percentage of errors for " + error.Key + ": " + Math.Round(errPercentage, 2) * 100 + "%");
        ordersPerHour = OrdersPerHour(error.Key);
        RandomOrders = RandomSelect(errPercentage, error.Key);


        Console.WriteLine("Number of orders pulled : " + RandomOrders.Rows.Count);
        //Print out orders randomly collected
        for (int i = 0; i < RandomOrders.Rows.Count; i++) 
        {
            Console.WriteLine(RandomOrders.Rows[i]["ControlNumber"]);
        }

        Console.WriteLine("\r\n");
        //NumOrdersToPull = FindNumOrdersToPull(Math.Round(errPercentage,2), ordersPerHour);
    }
}

Answer 4

Or even just one loop, and then a lookup for the other? is totalOrders a list of some kind? if you could turn one into a dictionary, you could do something like

foreach (KeyValuePair<string, int> total in totalOrders)
{
    // do work for each order
    object whatever;
    if (errorOrders.TryGetValue( total.Key, out whatever ))
    {
        // do extra work because this order has an error
    }
}

Answer 5

The other answers helped me reach this conclusion:

foreach (KeyValuePair<string, int> e in errorOrders)
{
    errPercentage = GetErrPercentage(e.Key);
    Console.WriteLine("Percentage of errors for " + e.Key + ": " + Math.Round(errPercentage, 2) * 100 + "%");
    ordersPerHour = OrdersPerHour(e.Key);
    RandomOrders = RandomSelect(errPercentage, e.Key);
}

Console.WriteLine("Number of orders pulled : " + RandomOrders.Rows.Count);
//Print out orders randomly collected
for (int i = 0; i < RandomOrders.Rows.Count; i++)
{
    Console.WriteLine(RandomOrders.Rows[i]["ControlNumber"]);
}
Console.WriteLine("\r\n");

static double GetErrPercentage(string user)
{
    double errPercentage = 0;
    errPercentage = (double)errorOrders[user]/ (double)totalOrders[user];          
    return errPercentage;
}