[QUESTION] Here are Common Mistakes When Working with Enumerables. Can you see it?

The following code contains some of the most common mistakes I have been seeing when reviewing code that deals with enumerable objects.

public static IEnumerable<T> ElementsFromTheFirstWhichAreAlsoInTheSecond<T>(
    IEnumerable<T> enumerable1,
    IEnumerable<T> enumerable2
)
{
    var result = new List<T>();

    enumerable1.ToList().ForEach(a =>
    {
        if (enumerable2.Contains(a))
        {
            result.Add(a);
        }
    });

    return result;
}

Can you see what is wrong? Share in the comments.

Compartilhe este insight:

8 respostas

  1. 1 – There is a List being created when calling ToList().
    2 – The Foreach is enumerating this list again.
    3 – There is a new list being returned.
    4 – This code is not using framework specific code for collections, that might get advantage of better way of dealingthis situation.
    5 – This can easily be replaced by: return first.Intersect(second);

      1. Interesting. As far as I understood, this method is returning objects that exist in both worlds.

        1. Here we go. This is the current underlying implementation of the Intersect function:

          private static IEnumerable<TSource> IntersectIterator<TSource>(
              IEnumerable<TSource> first, 
              IEnumerable<TSource> second, 
              IEqualityComparer<TSource> comparer
              )
          {
              Set<TSource> set = new Set<TSource>(comparer);
              foreach (TSource element in second)
              {
                  set.Add(element);
              }
          
              foreach (TSource element in first)
              {
                  if (set.Remove(element))
                  {
                      yield return element;
                  }
              }
          }
          

          Did you see the difference?

  2. For some reason a few developers like so much to write inline code. So, this pattern enumerable1.ToList().ForEach is very common. Problem is ToList will iterate all elements of enumerable in order to create a new object (more complex) List. Second problem is var result = new List(). The foreach will iterate all items of List and will add to result List object if condition is satisfied, so it could (and one day for sure will) explodes memory. Instead to add filtered item to a new object it should be yield returned, once the method returns IEnumerable. Last, regarding performance, if method handles large/complex data, a hashset will perform better to compare lists objects.

Deixe um comentário

O seu endereço de e-mail não será publicado. Campos obrigatórios são marcados com *

Elemar Júnior

Sou fundador e CEO da EximiaCo e atuo como tech trusted advisor ajudando diversas empresas a gerar mais resultados através da tecnologia.

Elemar Júnior

Sou fundador e CEO da EximiaCo e atuo como tech trusted advisor ajudando diversas empresas a gerar mais resultados através da tecnologia.

Mais insights para o seu negócio

Veja mais alguns estudos e reflexões que podem gerar alguns insights para o seu negócio:

Mudança nunca é fácil. Em uma organização, então, este tema costuma ser algo bem complicado. A execução da estratégia implica...
Em minhas consultorias, quando questionado sobre escalabilidade, recorro sempre ao scale cube, compartilhado no excelente livro “The Art of Scalability”,...
Nesse ano, palestrei na APIX sobre microsserviços. Abaixo, registro em vídeo feito pela organização do evento. Comentários? Feedback?
Implementar novas tecnologias implica na adoção de novos critérios e conhecimentos. Quando pensamos em bancos de dados, estamos tão habituados...
In the previous post, I mentioned Mario Fusco who wrote a blog post series questioning the way Java developers are...
Tem coisas que a gente até sabe, mas ignora pela vaidade… Uma das features mais aclamadas do Microsoft Teams, para...
× Precisa de ajuda?