Switch to lambda/Drum a foreach

Asked

Viewed 332 times

4

I have this method with a foreach inside

[Route("")]
        [HttpGet]
        [ResponseType(typeof(List<MarkupListResponse>))]
        public IHttpActionResult Get(int resellerId)
        {
            var catalogs = _catalogService.GetAllByResellerId(resellerId).ToList();
            var model = new List<MarkupListResponse>();
            foreach (var catalog in catalogs)
            {
                model.Add(new MarkupListResponse()
                {
                    CreatedOn = catalog.CatalogDate,
                    CatalogId = catalog.Id,
                    ItemsQuantity = catalog.Items.Count
                });
            }

            return Ok(model);
        }

In this foreach I have more than 400 items in Catalogs. As I do to pssar for a lambda, because I think it can improve the performance, I think.

I am using Entity Framework for these queries and MVC.

c# linq lambda-expressions
  • 3

    No, it will only make the performance worse. I already answered about, for you included: https://answall.com/a/53857/101

  • @Maniero, inside the foreach, I give one new, that is, instancio to each iteration the object Markuplistresponse and populate the properties of his. Can this interfere with performance? Because in this loop, I have a bad performance, among others in this project.

  • Yes, but you can’t always escape it. In some, yes, but there is room for a deep analysis. Maybe take this ToList() can help but do not think much. If it is bad, the problem should be another because 400 is little thing.

  • 1

    Start by optimizing your query in the database, return only the information you will use, already reflecting a List<MarkupListResponse> and check the indexes... It is much more likely that your bottleneck is at that point than in foreach...

  • @Leandroangelo, but I got a performance just by swapping the foreach for the lambda, according to my own answer. I will do this (I followed your guidance at the bank) in another bottleneck that I have, which originated my first post on this subject, even if it has achieved improvement, but it is still very slow, for the amount of items(202)

  • In what @Maniero commented, there in that post are saying that LINQ is slower than For/Foreach, but greatly improved the performance using Linq instead of foreach in my case, and now?

  • You are using EF?

  • @Maniero, yes I use EF

  • 2

    Then it is different, nor should have considered using otherwise, the EF will convert it to query instead of bringing it all in and processing it into memory. If you do not put this information fully relevant and necessary it becomes complicated to answer properly.

  • @Maniero, thanks. I really "ate a cap", "I stepped on the round".

Show 5 more comments

1 answer

3

I did that and I won a lot. Now you have about 3s to execute, which took more than 2min. 

var catalogs = _catalogService.GetAllByResellerId(resellerId).Select(c => new MarkupListResponse
            {
                CreatedOn = c.CatalogDate,
                CatalogId = c.Id,
                ItemsQuantity = c.Items.Count()
            }).ToList();

            return Ok(catalogs);
  • 3

    Understand, that in this case, it is most likely not lambda expression that improved your performance... but rather the query generated to query the database.

  • @Leandroangelo, it makes sense what you said, but these things are very conceptual, for example, to iterate and send a query to the bank, they say that Linq/Lambda is more performative, for what I have read. Iterate a list, I think the foreach/for is more performative, so I just read in the post of a friend of mine. Like they said, it depends on how and what to use.

  • 1

    Test, compare previous and current queries

Browser other questions tagged c# linq lambda-expressions

You are not signed in. Login or sign up in order to post.