none
More efficient code RRS feed

  • Question

  • Hi everybody,

    I have the following code which is quite straightforward:

    IList<SCatItmLnk> linksToInsert = new List<SCatItmLnk>();
                // Except returns a sequence that contains all the elements of
                // the first sequence that do not exist in the second sequence.
    
                IList<Int32> linksToAdd = assignedItems.Except(currentItems).ToList();
                foreach (Int32 itemId in linksToAdd)
                {
                    SCatItmLnk lnk = new SCatItmLnk()
                    {
                        SubCatId = id,
                        ItemId = itemId
                    };
                    linksToInsert.Add(lnk);
                }

    I am wondering if it's possible to re-write it to be simpler and avoid extra loop.

    Thanks in advance.


    For every expert, there is an equal and opposite expert. - Becker's Law


    My blog


    My TechNet articles


    • Edited by Naomi N Friday, July 17, 2015 8:24 PM
    Friday, July 17, 2015 8:23 PM

Answers

  • https://lostechies.com/jimmybogard/2008/05/09/linq-query-operators-lose-that-foreach-already/

    <copied>

    Lose the foreach

    With the new LINQ query operators, any temporary list creation and foreach loop should be considered suspect.  By understanding the operations LINQ gives us, we can not only reduce the amount of code we create, but the end result matches the original intent far better.

    <end>

    https://msdn.microsoft.com/en-us/library/ms973839.aspx?f=255&MSPPError=-2147217396

    <copied>

    Use AddRange to Add Groups

    Use AddRange to add a whole collection, rather than adding each item in the collection iteratively. Nearly all windows controls and collections have both Add and AddRange methods, and each is optimized for a different purpose. Add is useful for adding a single item, whereas AddRange has some extra overhead but wins out when adding multiple items. Here are just a few of the classes that support Add and AddRange:

    <end>

     

            public InboxController(IMessageServiceClient messageServiceClient, IEFilePrincipalContextService principalContextService)
            {
                _messageserviceclient = messageServiceClient;
                _principalContextService = principalContextService;
            }

            public dynamic Get([ModelBinder(typeof(Models.DataSourceRequestModelBinder))] DataSourceRequest request)
            {
                var list = _messageserviceclient.GetInboxMessages(request.Page, request.PageSize, EFilePrincipalHelper.GetUserId());
            
                var vmList = new List<InboxMessageModel>();
                if (list != null)
                {
                    vmList.AddRange(list.Select(item => new InboxMessageModel
                    {
                        MessageId = item.Id,
                        From = item.FromUser.Metadata["PrincipalName"] as string,
                        To = _principalContextService.GetUsername(),
                        Subject = item.Subject,
                        Body = item.Body,
                        Read = item.IsRead,
                        ReadTime = item.ReadTime,
                        SentTime = item.SentTime,
                        CanRespond = (Guid) item.FromUser.Id != SystemUser.Id
                    }));
                }

                return new { TotalRecords =  _messageserviceclient.GetInboxMessagesCount(EFilePrincipalHelper.GetUserId()), Records = vmList };

            }
        }

        

    • Marked as answer by Naomi N Monday, July 20, 2015 9:49 PM
    Friday, July 17, 2015 9:40 PM

All replies

  • https://lostechies.com/jimmybogard/2008/05/09/linq-query-operators-lose-that-foreach-already/

    <copied>

    Lose the foreach

    With the new LINQ query operators, any temporary list creation and foreach loop should be considered suspect.  By understanding the operations LINQ gives us, we can not only reduce the amount of code we create, but the end result matches the original intent far better.

    <end>

    https://msdn.microsoft.com/en-us/library/ms973839.aspx?f=255&MSPPError=-2147217396

    <copied>

    Use AddRange to Add Groups

    Use AddRange to add a whole collection, rather than adding each item in the collection iteratively. Nearly all windows controls and collections have both Add and AddRange methods, and each is optimized for a different purpose. Add is useful for adding a single item, whereas AddRange has some extra overhead but wins out when adding multiple items. Here are just a few of the classes that support Add and AddRange:

    <end>

     

            public InboxController(IMessageServiceClient messageServiceClient, IEFilePrincipalContextService principalContextService)
            {
                _messageserviceclient = messageServiceClient;
                _principalContextService = principalContextService;
            }

            public dynamic Get([ModelBinder(typeof(Models.DataSourceRequestModelBinder))] DataSourceRequest request)
            {
                var list = _messageserviceclient.GetInboxMessages(request.Page, request.PageSize, EFilePrincipalHelper.GetUserId());
            
                var vmList = new List<InboxMessageModel>();
                if (list != null)
                {
                    vmList.AddRange(list.Select(item => new InboxMessageModel
                    {
                        MessageId = item.Id,
                        From = item.FromUser.Metadata["PrincipalName"] as string,
                        To = _principalContextService.GetUsername(),
                        Subject = item.Subject,
                        Body = item.Body,
                        Read = item.IsRead,
                        ReadTime = item.ReadTime,
                        SentTime = item.SentTime,
                        CanRespond = (Guid) item.FromUser.Id != SystemUser.Id
                    }));
                }

                return new { TotalRecords =  _messageserviceclient.GetInboxMessagesCount(EFilePrincipalHelper.GetUserId()), Records = vmList };

            }
        }

        

    • Marked as answer by Naomi N Monday, July 20, 2015 9:49 PM
    Friday, July 17, 2015 9:40 PM
  • Sorry for not replying sooner. I eventually got that to work using a simpler code and also found a problem in my original implementation that I corrected since. My current code is

     private void RemoveItemsFromSubCats(int id, IEnumerable<SCatItmLnk> currentItems, IList<int> assignedItems)
            {
                IEnumerable<SCatItmLnk> linksToDelete = from s in currentItems where !assignedItems.Any(it=>it==s.ItemId) select s;          
    
                _sCatItmLnkRepository.Delete(linksToDelete);
            }
    
            private void AssignItemsToSubCats(int id, IEnumerable<SCatItmLnk> currentItems, IList<int> assignedItems)
            {
                IList<int> currentItemIds = currentItems.Select(it=>it.ItemId).ToList();
    
                // Except returns a sequence that contains all the elements of
                // the first sequence that do not exist in the second sequence.
                IList<SCatItmLnk> linksToInsert = assignedItems.Except(currentItemIds).Select(x=> new SCatItmLnk() { SubCatId = id, ItemId = x} ).ToList();
      
                _sCatItmLnkRepository.Add(linksToInsert);
            }


    For every expert, there is an equal and opposite expert. - Becker's Law


    My blog


    My TechNet articles

    Monday, July 20, 2015 9:52 PM