Getting list of daily team goals

The old code I had before was atrociously slow but after some advice and research I was able to take a 2-5 minutes run time down to about 5-30 seconds. That is acceptable, but still looking to have it run as quickly as possible.

I'm still cleaning it up, like there shouldn't be two seperate PP entities calls.

public static List GetListDailyTeamGoals(int teamId)
{
    string teamGoal = "";
    List lstProPit_User = new List();
    using (PPEntities db = new PPEntities())
    {
        Team team = db.Teams.Where(x => x.teamID == teamId).FirstOrDefault();

        if (team != null)
        {
            teamGoal = Convert.ToString(team.goal);
        }

        lstProPit_User = db.ProPit_User.Where(x => x.teamID == teamId).ToList();
    }

    List lstDailyTeamGoal = new List();
    using (TEntities db = new TEntities())
    using (PPEntities ppdb = new PPEntities())
    {
        //have to get every day of the month
        DateTime dt = DateTime.Now;
        int days = DateTime.DaysInMonth(dt.Year, dt.Month);
        decimal orderTotal = 0m;
        for (int day = 1; day  o.DateCompleted >= dtStartDate
                                                && o.DateCompleted  x.teamID == teamId).Select(v => v.SalesRepID).ToList();

            var total = (from o in ordersQuery
                            where lstSalesRepIDs.Contains(o.SalesRepID)
                            select o.OrderTotal).Sum();

            orderTotal += total;
            dtg.DailyTotal = orderTotal;

            lstDailyTeamGoal.Add(dtg);
        }
    }

    return lstDailyTeamGoal;
}

You are calling queries in a for-loop for each day. Database queries are slow compared to C# code execution. Send only one query to the DB for the whole range of days.

You can do the grouping by day either as DB query or query the ungrouped records and group using LINQ-to-Objects later.

Also you are querying the lstSalesRepIDs within the loop. Why? The teamId seems not to change between the loop iterations. Query it before the loop.

Also your where-clause tests the days like this: o => o.DateCompleted >= dtStartDate && o.DateCompleted = dtStartDate && o.DateCompleted < dtEndDate. (Note: I used < instead of <= for the upper bound.)

Don't make date calculations by converting from DateTime to string and back! DateTime contains all the functionality you need, without using any dirty tricks.

var salesRepIDs = new HashSet(ppdb.ProPit_User
    .Where(ppu => ppu.teamID == teamId)
    .Select(ppu => ppu.SalesRepID)
    .AsEnumerable()); // HashSet.Contains is faster than List.Contains.

DateTime today = DateTime.Today;
DateTime startDate = new DateTime(today.Year, today.Month, 1);
DateTime endDate = today.AddDays(1);

// Note: If the DB contains no entries with future dates, 
//       you can drop the test for the upper date!
var orders = db.Orders
    .Where(o => o.DateCompleted >= startDate
                && o.DateCompleted  salesRepIDs.Contains(o.SalesRepID))
    .GroupBy(o => o.DateCompleted.Date); // .Date in order to drop the time part.
foreach (var dayGroup in ordersByCompletionDate) {
    decimal daylyTotal = dayGroup.Sum(o => o.OrderTotal);
    // ...
}

.AsEnumerable() has the advantage over .ToList() that the source is consumed on the fly, item by item. No in-memory list needs to be created.

The salesRepIDs.Contains(..) part requires special attention. It could be performed as part of the DB query; however depending on the implementation and the number of salesRepIDs this could create queries with long IN lists: WHERE id IN (1, 2, 3, ..). There is a limit for the length of these lists. This can lead to "Query too complex" exceptions. If this is not the case, i.e., if you know that you will always have a reasonable number of ID's, could can simplify the whole code and do everything (filtering, grouping and summing) with only one single DB query.

StackExchange Code Review Q#48082, answer score: 8