how to optimize the code to improve performance foreach loop in C#?

25 views Asked by At

Below functionality is taking more time iteration, It's required to optimize code to improve the Performance to fetch data . can you help me how to optimize ?

private List<ApplicationMenuItemDTO> toDTO(List<ApplicationMenuItem> list)
{
    if (list.Count() > 0)
    {
        ApplicationUser u = manager.FindById(User.Identity.GetUserId<int>());
        List<ApplicationMenuItemDTO> miDTOs = new List<ApplicationMenuItemDTO>();
        foreach(ApplicationMenuItem mi in list)
        {
            if (mi.PermissionId == null // menu items that do not require permission
                || u.HasPermission(mi.PermissionId)) // menu items user has permission to
            {
                miDTOs.Add(new ApplicationMenuItemDTO {
                    MenuItemId = mi.MenuItemId,
                    ParentMenuItemId = mi.ParentMenuItemId,
                    Name = mi.Name,
                    NamePlural = mi.NamePlural,
                    DisplayName = mi.DisplayName,
                    DisplayNamePlural = mi.DisplayNamePlural,
                    DeepLink = mi.DeepLink,
                    ApplicationPageId = mi.ApplicationPageId,
                    Description = mi.Description,
                    PermissionId = mi.PermissionId,
                    IsLocked = mi.IsLocked,
                    Url = mi.Url,
                    Rank = mi.Rank,
                    MenuItemTypeID = mi.MenuItemTypeID,
                    MenuItem = toDTO(db.ApplicationMenuItems.Where(x => x.ParentMenuItemId == mi.MenuItemId).ToList())
                });
            }
        }

        return miDTOs.OrderBy(x=>x.Rank).ToList();
    }
    else
    {
        //base case - return empty list
        return new List<ApplicationMenuItemDTO>();
    }
}
1

There are 1 answers

0
Mark Seemann On

It's difficult to tell, because the OP doesn't write where the performance problems are, but here are a couple of guesses.

ApplicationUser u = manager.FindById(User.Identity.GetUserId<int>());

I can't tell whether this query is expensive, but on the other hand, it doesn't seem to rely on any mi data. Thus, you could move it out of the foreach loop.

This line of code:

MenuItem = toDTO(db.ApplicationMenuItems
    .Where(x => x.ParentMenuItemId == mi.MenuItemId)
    .ToList())

looks as though it performs a database query per loop iteration. Thus, if you have fifty items in list, you'd be performing fifty database queries.

Try to investigate whether you can move database queries out of the loop, and instead make a single bulk query. Then zip or join the result of that query with list.