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:
- The result count of the current answer.
- 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);
}