user8512043
user8512043

Reputation: 1157

Optimize Code Snippet Skipping Loop Iteration

I've the below method to get user group data. The list of ids I am passing as follows:

Input:

var list = new List<int> { 1002, 1004, 1006, 1008 };

Method:

public async Task<ResponseBase> GetUserGroups(List<int> userId)
{
     var groups = await _userRepository.GetUserGroupsToDownload();
     var userGroups = await _userRepository.GetUserGroupsToDownload(userId);

     List<UserGroupDto> aLst = new List<UserGroupDto>();
     List<int> ids = new List<int>();

     //Found ids from userGroups and add it to a list
     foreach (var group in userGroups)
     {
         var result = new UserGroupDto();
         result.GroupId = group.GroupId;
         result.Id = group.UserId;
         result.EnterpriseId = group.EnterpriseId;
         result.Name = groups.Where(c => c.Id == group.GroupId).FirstOrDefault().Name;
         result.Enabled = userGroups.Any(ug => ug.GroupId == group.GroupId);

         aLst.Add(result);
         ids.Add(group.UserId); //Keeping track ids that are found to eliminate in the second loop
     }

     //Add ids in the list that aren't in the userGroups    
     foreach (var id in userId.Where(c => !ids.Contains(c)))
     {
         var result = new UserGroupDto();
         result.GroupId = 0;
         result.Id = id;
         result.EnterpriseId = 0;
         result.Name = "";
         result.Enabled = false;

         aLst.Add(result);
     }

     return ResponseBase.WithSuccess().WithData(aLst);
}

Now the thing is, as an example if the groups are found for 1002, 1004, then at the same time I've to get the data for 1006 and 1008 though they don't belong to any group. To do this, I've a second loop that I am not preferring to use. So is there any alternate or bit faster way to eliminate the second loop and assign default values to those ids that doesn't have any group assigned?

Upvotes: 0

Views: 88

Answers (1)

T N
T N

Reputation: 10205

I believe you can achieve what you want using LINQ and a combination of the .SelectMany() and .DefaultIfEmpty() methods.

    List<UserGroupDto> aLst =
        userId  // This is a list
        .SelectMany(id =>
            userGroups
                .Where(group => group.UserId = id)
                .DefaultIfEmpty()  // Inject a null value if no results
                .Select(group => new UserGroupDto {
                    GroupId = group?.GroupId ?? 0,
                    Id = id,
                    EnterpriseId = group?.EnterpriseId ?? 0,
                    Name = group == null ? "" :
                        groups.Where(c => c.Id == group.GroupId).FirstOrDefault().Name,
                    Enabled = group == null ? false :
                        userGroups.Any(ug => ug.GroupId == group.GroupId)
                    //Enabled = group != null,  // Maybe?
                })
       )
       .ToList();

For each UserId, the above will extract the those userGroups objects that match. If there are no matches, the .DefaultIfEmpty() function will inject a null object into the sequence. That sequence is then converted to the final UserGroupDto with care to handle the null objects using a combination of null propagation (?.) and coalesce (??) operators. In some cases the terinary case ?: operator is used.

If you prefer to separate the match and non-match DTO creation logic, you could use the following alternative:

    List<UserGroupDto> aLst =
        userId  // This is a list
        .SelectMany(id =>
            userGroups
                .Where(group => group.UserId = id)
                .Select(group => new UserGroupDto {
                    GroupId = group.GroupId,
                    Id = group.UserId,
                    EnterpriseId = group.EnterpriseId,
                    Name = groups.Where(c => c.Id == group.GroupId).FirstOrDefault().Name,
                    Enabled = userGroups.Any(ug => ug.GroupId == group.GroupId)
                })
                .DefaultIfEmpty(new UserGroupDto {
                    GroupId = 0,
                    Id = id,
                    EnterpriseId = 0,
                    Name = "",
                    Enabled = false
                })
       )
       .ToList();

Although longer, this might be considered more readable.

I believe the .GroupJoin() method can also be used, and may be more efficient if the collection is large. Under the covers, .GroupJoin() uses a hash table under the covers to perform the match operation.

The result (after applying the result mapping function provided as the 4th parameter) is an enumeration of objects that each contain a userId plus a (possibly empty) list of matching UserGroup objects.

    List<UserGroupDto> aLst =
        userId
        .GroupJoin(userGroups, id => id, ug => ug.UserId, (uid, ugs) => new { id, ugs })
        .SelectMany(
            item => item.ugs
                .DefaultIfEmpty(),
                .Select(group => new UserGroupDto {
                    GroupId = group?.GroupId ?? 0,
                    Id = item.id,
                    EnterpriseId = group?.EnterpriseId ?? 0,
                    Name = group == null ? "" :
                        groups.Where(c => c.Id == group.GroupId).FirstOrDefault().Name,
                    Enabled = group == null ? false :
                        userGroups.Any(ug => ug.GroupId == group.GroupId)
                    //Enabled = group != null,
                })
        )
        .ToList();

Or:

    List<UserGroupDto> aLst =
        userId
        .GroupJoin(userGroups, id => id, ug => ug.UserId, (id, ugs) => new { id, ugs })
        .SelectMany(
            item => item.ugs
                .Select(group => new UserGroupDto {
                    GroupId = group.GroupId,
                    Id = group.UserId, // Same as item.id
                    EnterpriseId = userGroup.EnterpriseId,
                    Name = groups.Where(g => g.Id == group.GroupId).FirstOrDefault().Name,
                    Enabled = userGroups.Any(ug => ug.GroupId == group.GroupId),
                    //Enabled = true, // Maybe
                })
                .DefaultIfEmpty(new UserGroupDto {
                    GroupId = 0,
                    Id = item.id,
                    EnterpriseId = 0,
                    Name = "",
                    Enabled = false,
                })
        )
        .ToList();

Upvotes: 1

Related Questions