Question Same query fired multiple times EF

[Matthieu]

I'm building a very simple CRUD web-application (ASP.NET MVC) for tennisplayers and the tournaments they can participate.
On a specific page I want to show all tournaments in the database with a title at the top op the page 'All Tournaments' with between brackets the amount of records in the database.
My cshtml would look like this:

@model System.Collections.Generic.IEnumerable<TMS.BL.Domain.Tournament>

@{
    ViewBag.Title = "All Tournaments";
    Layout = "_Layout";
}
    <h3>All Tournaments (@Model.Count())</h3>
    
    @if (!@Model.Any())
{
    <p>No tournaments were found...</p>
}
else
{
    <table class="table">
        <thead>
        <tr>
            <th scope="col">Name</th>
            <th scope="col">Starts</th>
            <th scope="col">Ends</th>
            <th scope="col">Org. Club</th>
            <th scope="col"></th>
        </tr>
        </thead>
        <tbody>
        @foreach (var t in Model)
        {
            <tr>
                <td>@t.Name</td>
                <td>@t.StartDate.ToString("ddd, dd/MM/yyyy")</td>
                <td>@t.EndDate.ToString("ddd, dd/MM/yyyy")</td>
                <td>@t.OrganizingClub.Name (@t.OrganizingClub.Province - @t.OrganizingClub.Town)</td>
                <td>
                    <a asp-controller="Tournament" asp-action="Details" asp-route-id="@t.Id" class="btn btn-primary btn-sm">Details</a>
                </td>
            </tr>
        }
        </tbody>
    </table>
}
}

The controller for this page is the TournamentController. This controller is using a Manager object which contains the dbcontext. The GetAllTournamentsWithOrgClubAndParticipants() method returns an IEnumerable of tournament objects (with an include of the club and pariticpants, but for my question this is not of any importance).

    public class TournamentController : Controller
    {
        private IManager _mgr;

        public TournamentController(IManager manager)
        {
            _mgr = manager;
        }

        public IActionResult Index()
        {
            return View(_mgr.GetAllTournamentsWithOrgClubAndParticipants());
        }

When I load the page, I see that the same query is fired 3 times. once for the @Model.Count() in the title of the webpage, once for the @Model.Any() to determine wheter or not to show the table and once in the foreach loop. Now I know this is because of defered excecution and I could resolve this by adding a ToList() behind the GetAllTournamentsWithOrgClubAndParticipants() in the controller class, but I often hear NOT TO USE the ToList() method because of this way you are loading everything into memory. But to my feeling for this case it's still better than excecuting the same query 3 times in a row or am I wrong? Any other way I could resolve this?

Thank you very much!

 

[jmcilhinney]

The problem is that LINQ queries are not actualised. They are basically instructions on how to get data, not actual data. Your GetAllTournamentsWithOrgClubAndParticipants method is returning an unactualised LINQ query and the actualisation takes place in each of the three places that you use that query.

The solution is to actualise the query in the controller action and then pass that actual data to your view:

return View(_mgr.GetAllTournamentsWithOrgClubAndParticipants().ToArray());

The ToArray call actualises the LINQ query, which executes the database query and loads the results into an array. Your view will then access that array three times. ALWAYS be mindful of when you're using an unactualised query and when you're using actual results. Methods like ToList, First and FirstOrDefault are also often used to actualise a query.

 

[Matthieu]

The problem is that LINQ queries are not actualised. They are basically instructions on how to get data, not actual data. Your GetAllTournamentsWithOrgClubAndParticipants method is returning an unactualised LINQ query and the actualisation takes place in each of the three places that you use that query.

The solution is to actualise the query in the controller action and then pass that actual data to your view:

return View(_mgr.GetAllTournamentsWithOrgClubAndParticipants().ToArray());

The ToArray call actualises the LINQ query, which executes the database query and loads the results into an array. Your view will then access that array three times. ALWAYS be mindful of when you're using an unactualised query and when you're using actual results. Methods like ToList, First and FirstOrDefault are also often used to actualise a query.

Thanks for the explanation! So as you suggest, I could use ToArray(), or ToList() and this wil execute the query one time and then this generared array is used in the view. But it's not that this is bad practice or something?

 

[jmcilhinney]

But it's not that this is bad practice or something?

What would be the rationale for that, given that the alternative is to execute the same database query three times? No matter what LINQ provider you're using, it's ALWAYS bad form to enumerate the same LINQ query multiple times. If you need random access to the data (more than once) or to enumerate it multiple times then you should ALWAYS call ToArray or ToList first and use the result.

 

[Skydiver]

Also consider changing your UI and logic around a little bit.

Change the check for Any() first. Any() will query and stop after the first item is found. If no items are found, you automatically know that Count() will be zero. If it's non-zero, then iterate over each item found, and keep a running count. So you'll query the database again, but this time actually go over all the items found. There's no need to call Count() to get the number of items since you've seen all the items go by and have that count readily available to you.

 

[Matthieu]

What would be the rationale for that, given that the alternative is to execute the same database query three times? No matter what LINQ provider you're using, it's ALWAYS bad form to enumerate the same LINQ query multiple times. If you need random access to the data (more than once) or to enumerate it multiple times then you should ALWAYS call ToArray or ToList first and use the result.

The only thing is, assume this is a massive database table, then it's copying the entire table into memory. Isn't this very bad performance-wise?

Probably still better than letting it excecute the same query 3 times, cause in theory it's possible that between excecution of the queries the table gets altered. So it possible that the count methos gives for example 5 back, but the foreach a little bit later loops over 6 records in case someone added a tournament in the meantime.

 

[jmcilhinney]

Isn't this very bad performance-wise?

What do you think is going to happen when you loop through the data in the view? How is all the data going to be displayed in the view without being loaded into memory?

I guess the question is, if there is so much data, why are you displaying all of it at the same time to begin with? Why aren't you paging and just displaying a subset of the data at any one time?