3
\$\begingroup\$

I've got an MVC APU project that I'm working on to feed data to some JavaScript charts. This beast below takes nearly 1 second of server time to process before it is served to the chart. I know that it's because of the nested for loops.

The chart is a line. By default it shows one data point per month for the last 5 years (one line per year, up to 12 points per line).

The only way I can think to do this is to say for each year, for each month add a point.

Is there a more efficient way to get this done? The resulting JSON looks like this:

{"YearLabel":2011,"MonthProduction":7505.83,"MonthNumber":1}

public string DealsPerMonth(int? BrokerID, int NumberOfYears, int? StartingYear)
    {
        DBNAME db = new DBNAME();

        List<NumberOfDealsPerMonth> dealsPerMonth = new List<NumberOfDealsPerMonth>();

        StartingYear = StartingYear ?? DateTime.Now.Year;

        for (var i = NumberOfYears; i > 0; i--)
        {
            var year = StartingYear - (i - 1);

            var deals = db.VDealParticipations.Where(w => w.IsPrimaryParticipant && w.EffectiveDate.Value.Year == year);

            if (BrokerID != null)
                deals = deals.Where(w => w.BrokerID == BrokerID);

            for (var month = 1; month <= 12; month++)
            {
                if (!(year == DateTime.Now.Year && month > DateTime.Now.Month))
                {
                    var dealCount = deals.Count(w => w.EffectiveDate.Value.Month == month);
                    dealsPerMonth.Add(new NumberOfDealsPerMonth
                    {
                        YearLabel = (int) year,
                        DealCount = dealCount,
                        MonthNumber = month
                    });
                }
            }
        }
        var serializer = new JavaScriptSerializer();
        var value = serializer.Serialize(dealsPerMonth.ToArray());
        return value;
    }

The class that's being used is as follows, although I am probably going to delete it as I found out about anonymous return types and so instead of

select(s => new NumberOfDealsPerMonth {blah = s.bleh}

I can do

select(s => new {blah = s.bleh}

and it does the same thing. But I want to include it anyway just for the sake of more information.

public class NumberOfDealsPerMonth
{
    public int YearLabel { get; set; }
    public int DealCount { get; set; }
    public int MonthNumber { get; set; }
}
\$\endgroup\$
1
  • 1
    \$\begingroup\$ As improving performance is a common request, please retitle the question to reflect the purpose of the code, not how you want to wish to transform it. See How to Ask. \$\endgroup\$ Apr 27, 2015 at 21:31

2 Answers 2

1
\$\begingroup\$

some suggestion:

  • use stored proc to populate your result instead of entity framework. EF will add some overhead when generating query. Counting on select statement will be faster, as the whole entity list isn't really necessary.
  • Use cache to save access to database unless your report should reflect real time data.
\$\endgroup\$
1
  • \$\begingroup\$ Hi jwei, thanks for your reply. I ended up needing to do it in a stored proc. I was holding it as a last resort but it worked well and I need to get better with sql anyway, so I got a DBA friend to help me. I would upvote your answer but I need 15 rep or more, sorry about that. \$\endgroup\$
    – gnnwd
    Apr 28, 2015 at 21:48
1
\$\begingroup\$

I think you will see a performance improvement by using GroupBy.

Let's start by calculating the earliest year we're interested in:

startingYear = startingYear ?? DateTime.Now.Year;
var minYear = startingYear - numberOfYears + 1;

Then filter the rows

var deals = db.VDealParticipations
    .Where(deal => deal.IsPrimaryParticipant &&
                   deal.EffectiveDate.Value.Year >= minYear &&
                   deal.EffectiveDate.Value.Year <= startingYear);
if (brokerID.HasValue)
{
    deals = deals.Where(deal => deal.BrokerID == brokerID);
}

Finally we group the rows by their year/month.

var dealsPerMonth = deals
    .GroupBy(deal => new { deal.EffectiveDate.Value.Year, deal.EffectiveDate.Value.Month })
    .Select(g => new { g.Key.Year, g.Key.Month, Deals = g.Count() });
return new JavaScriptSerializer().Serialize(dealsPerMonth.ToArray());

Note that this does not return months with a deal count of 0.

\$\endgroup\$
1
  • \$\begingroup\$ Hi mjolka, thanks for your reply. I really love this method and will be using it in future projects for sure. I do need zero months though so it ended up not working for me. I tried compensating for it but it was getting ugly fast. I went with the stored proc suggestion with the help of my DBA friend. Thanks! I would upvote your answer but I need 15 rep or more, sorry about that. \$\endgroup\$
    – gnnwd
    Apr 28, 2015 at 21:47

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.