Efficiency of a linq query on a nested structure

Question

For a survey website I've written this query to show the survey results, with a percentage for each chosen answer:

SurveyResultViewModel srvm =
    (from s in _context.Surveys
        where s.Id == id

        select new SurveyResultViewModel
        {
            Title = s.Title,
            ResponseCount = s.Results.Count,
            Questions = from q in s.Questions
                select new QuestionResultViewModel
                {
                    Text = q.Text,
                    Answers = from a in q.Answers
                        select new AnswerResultViewModel
                        {
                            Text = a.Text,
                            SelectedCount = a.Results.Count,

                            SelectedPercentage = 
                            (double)a.Results.Count / q.Answers.Select(an => an.Results.Count).Sum()
                        }
                }
        }).Single();                                                                              

However, I do think it can be improved, just not sure how. It runs 4 queries for every question in the survey because of the line that calculates the percentage.

Answer 1

It runs 4 queries for every question in the survey because of the line that calculates the percentage.

You're trying to retrieve data and make calculations at the same time. The issue stems from trying to do these two things. This is basically violating SRP.

Edit According to a coworker, this is not strictly an SRP violation, but it's very close to the spirit of what makes SRP important. I sort of see his point, but an in-depth discussion is a bit offtopic here.

Instead, try to only retrieve data at this point. There are other ways to do the calculation at a later stage:

• Use an automated property
• Perform the calculation on the in-memory objects

---

1. Only retrieve the needed information and store it in an appropriate spot.

We have to make sure that we retrieve all needed information that allows us to do the calculation at a later stage. Your percentage calculation relies on two pieces of information:

1. The result count of the current answer.
2. The combined result count of all answers that belong to the (parent) question.

(1) is already being retrieved (it's stored in SelectedCount).

(2) is not explicitly retrieved, but can already be derived by looking at the Answers property of a QuestionResultViewModel. In short, it can be found by calling myQuestionModel.Answers.Sum(x => x.SelectedCount)

So the first step is easy, simply omit the calculation:

// ...

Answers = from a in q.Answers
    select new AnswerResultViewModel
    {
        Text = a.Text,
        SelectedCount = a.Results.Count
    }

// ...

---

2. Option 1 - using an automated property.

I find this the cleanest solution, it forgoes the need to manually trigger a calculation. There are a few prerequisites here:

• An AnswerResultViewModel needs to have a reference to its parent QuestionResultViewModel in order to access the total count. For the example, I will assume such a property exists by the name of Question.
• If the calculation is expensive, you should reconsider using an automated property because it will be recalculated every time it is accessed. However, I don't consider your current example to be an expensive calculation.

The automated property, using an expression-bodied member:

public int SelectedPercentage => 
               (double)this.SelectedCount / this.Question.Answers.Sum(an => an.SelectedCount);

If you prefer a more traditional syntax:

public int SelectedPercentage
{
    get
    {
        return (double)this.SelectedCount / this.Question.Answers.Sum(an => an.SelectedCount);
    }
}

Minor changes:

• Instead of .Select(lambda).Sum(), I simply used .Sum(lambda). It's slightly shorter and easier to read.
• Instead of an.Results.Count, I used an.SelectedCount since it already contains the correct value.

---

3. Option 2 - Manual calculation.

I like this option less, but it's better in cases where the calculation is expensive and you want to avoid calculating the same value over and over.

After you've retrieved the value from the database, calculate the percentages.

If the Question navigational property exists:

foreach(var answer in srvm.Questions.SelectMany(q => q.Answers))
{
    answer.SelectedPercentage = (double)answer.SelectedCount / answer.Question.Answers.Sum(an => an.SelectedCount);
}

If there is no navigational property:

foreach(var question in srvm.Questions)
foreach(var answer in question.Answers)
{
    answer.SelectedPercentage = (double)answer.SelectedCount / question.Answers.Sum(an => an.SelectedCount);
}

Answer 2

Keep your database access as short (queries and time) as possible. There are two main reasons to do that:

1. Detach how and where you get the info from (tomorrow it may come from the network... ot another method) and what you do with that info.
2. Don’t get the database too busy with transactions, making it not able to handle properly requests from other methods.

I'd do it something similar to:

var survey = _context.Surveys
.Include(x => x.Questions.Select(y => y.Answers).Select(z => z.Results))) // Retrive it now so you don't go to database per each question/answer/result
.First(x => x.ID == id); // Sometimes, you don't need to prove there's only one survey with that id

After that call, you can manage your logic without bothering the database.

As others have advised to you, use the SQL Profiler to check how many queries are actually made.