Tratamento de aviso para possível enumeração múltipla de IEnumerable

359

No meu código na necessidade de usar IEnumerable<>várias vezes, portanto, obter o erro Resharper de "Enumeração múltipla possível de IEnumerable".

Código de amostra:

public List<object> Foo(IEnumerable<object> objects)
{
    if (objects == null || !objects.Any())
        throw new ArgumentException();

    var firstObject = objects.First();
    var list = DoSomeThing(firstObject);        
    var secondList = DoSomeThingElse(objects);
    list.AddRange(secondList);

    return list;
}
  • Posso alterar o objectsparâmetro Liste evitar a possível enumeração múltipla, mas não obtenho o objeto mais alto que posso manipular.
  • Outra coisa que posso fazer é converter o IEnumerablepara Listno início do método:

 public List<object> Foo(IEnumerable<object> objects)
 {
    var objectList = objects.ToList();
    // ...
 }

Mas isso é apenas estranho .

O que você faria neste cenário?

gdoron está apoiando Monica
fonte

Respostas:

471

O problema de usar IEnumerablecomo parâmetro é que ele diz aos chamadores "desejo enumerar isso". Não diz a eles quantas vezes você deseja enumerar.

Posso alterar o parâmetro objects para List e evitar a possível enumeração múltipla, mas não obtenho o objeto mais alto que posso manipular .

O objetivo de pegar o objeto mais alto é nobre, mas deixa espaço para muitas suposições. Deseja realmente que alguém passe uma consulta LINQ to SQL para esse método, apenas para enumerá-la duas vezes (obtendo resultados potencialmente diferentes a cada vez?)

A falta semântica aqui é que um chamador, que talvez não tenha tempo para ler os detalhes do método, pode presumir que você repete apenas uma vez - para que ele passe um objeto caro para você. A assinatura do seu método não indica os dois lados.

Ao alterar a assinatura do método para IList/ ICollection, você pelo menos esclarecerá ao chamador quais são suas expectativas e elas podem evitar erros dispendiosos.

Caso contrário, a maioria dos desenvolvedores que analisam o método pode presumir que você itera apenas uma vez. Se tomar um IEnumerableé tão importante, considere fazê-lo .ToList()no início do método.

É uma pena que o .NET não tenha uma interface que seja IEnumerable + Count + Indexer, sem os métodos Adicionar / Remover etc., que é o que eu suspeito que resolveria esse problema.

Paul Stovell
fonte
32
O ReadOnlyCollection <T> atende aos requisitos de interface desejados? msdn.microsoft.com/pt-br/library/ms132474.aspx
Dan está
74
@ DanNeely eu sugeriria IReadOnlyCollection(T)(novo no .net 4.5) como a melhor interface para transmitir a idéia de que é uma IEnumerable(T)que deve ser enumerada várias vezes. Como esta resposta afirma, IEnumerable(T)ela é tão genérica que pode até se referir a conteúdo não redefinível que não pode ser enumerado novamente sem efeitos colaterais. Mas IReadOnlyCollection(T)implica re-iterabilidade.
binki
11
Se você fizer isso .ToList()no início do método, primeiro precisará verificar se o parâmetro não é nulo. Isso significa que você obterá dois lugares onde você lançará uma ArgumentException: se o parâmetro for nulo e se não houver nenhum item.
Comecme
Li este artigo sobre a semântica de IReadOnlyCollection<T>vs IEnumerable<T>e postei uma pergunta sobre o uso IReadOnlyCollection<T>como parâmetro e em quais casos. Penso que a conclusão a que cheguei é a lógica a seguir. Que ele deve ser usado onde a enumeração é esperada, não necessariamente onde possa haver enumeração múltipla.
Neo
32

Se seus dados sempre serão repetíveis, talvez não se preocupe. No entanto, você também pode desenrolá-lo - isso é especialmente útil se os dados recebidos puderem ser grandes (por exemplo, leitura do disco / rede):

if(objects == null) throw new ArgumentException();
using(var iter = objects.GetEnumerator()) {
    if(!iter.MoveNext()) throw new ArgumentException();

    var firstObject = iter.Current;
    var list = DoSomeThing(firstObject);  

    while(iter.MoveNext()) {
        list.Add(DoSomeThingElse(iter.Current));
    }
    return list;
}

Observe que mudei um pouco a semântica de DoSomethingElse, mas isso é principalmente para mostrar o uso desenrolado. Você pode envolver novamente o iterador, por exemplo. Você também pode transformá-lo em um bloco iterador, o que pode ser bom; então não há list- e você faria yield returnos itens à medida que os obtém, em vez de adicionar a uma lista a ser devolvida.

Marc Gravell
fonte
4
+1 Bem, esse é um código muito bom, mas eu perco a beleza e a legibilidade do código.
Gdoron está apoiando Monica
5
@ Gdoron nem tudo é bonito; p Eu não o acima é ilegível, no entanto. Especialmente se for transformado em um bloco iterador - bastante fofo, IMO.
Marc Gravell
Eu posso apenas escrever: "var objectList = objects.ToList ();" e salve toda a manipulação do Enumerator.
Gdoron está apoiando Monica
10
@gdoron na pergunta que você disse explicitamente que realmente não queria fazer isso; p Além disso, uma abordagem ToList () pode não ser adequada se os dados forem muito grandes (potencialmente infinitos - seqüências não precisam ser finitas, mas podem ainda seja iterado). Por fim, estou apenas apresentando uma opção. Somente você conhece o contexto completo para ver se é justificado. Se seus dados são repetíveis, outra opção válida é: não altere nada! Ignore o R # em vez disso ... tudo depende do contexto.
Marc Gravell
11
Eu acho que a solução de Marc é bastante agradável. 1. É muito claro como a 'lista' é inicializada, é muito claro como a lista é iterada e é muito claro quais membros da lista são operados.
Keith Hoffman
9

O uso IReadOnlyCollection<T>ou IReadOnlyList<T>na assinatura do método, em vez de IEnumerable<T>, tem a vantagem de tornar explícito que você pode precisar verificar a contagem antes de iterar ou iterar várias vezes por algum outro motivo.

No entanto, eles têm uma enorme desvantagem que causará problemas se você tentar refatorar seu código para usar interfaces, por exemplo, para torná-lo mais testável e amigável ao proxy dinâmico. O ponto principal é que IList<T>não herda deIReadOnlyList<T> e da mesma forma para outras coleções e suas respectivas interfaces somente leitura. (Em resumo, isso ocorre porque o .NET 4.5 queria manter a compatibilidade ABI com versões anteriores. Mas eles nem aproveitaram a oportunidade para mudar isso no núcleo do .NET. )

Isso significa que, se você obtém uma IList<T>parte de algum programa e deseja passá-lo para outra parte que espera uma IReadOnlyList<T>, não pode! No entanto, você pode passar um IList<T>como umIEnumerable<T> .

No final, IEnumerable<T>é a única interface somente leitura suportada por todas as coleções .NET, incluindo todas as interfaces de coleção. Qualquer outra alternativa voltará a mordê-lo quando você perceber que se excluiu de algumas opções de arquitetura. Então eu acho que é o tipo apropriado para usar em assinaturas de funções para expressar que você deseja apenas uma coleção somente leitura.

(Observe que você sempre pode escrever um IReadOnlyList<T> ToReadOnly<T>(this IList<T> list)método de extensão que faça a conversão simples, se o tipo subjacente suportar as duas interfaces, mas você deve adicioná-lo manualmente em qualquer lugar ao refatorar, onde IEnumerable<T>sempre é compatível.)

Como sempre, isso não é absoluto; se você estiver escrevendo um código pesado para o banco de dados, em que a enumeração múltipla acidental seria um desastre, talvez prefira uma troca diferente.

Gabriel Morin
fonte
2
+1 Para chegando em um post antigo e adicionando documentação sobre novas formas de resolver este problema com os novos recursos fornecidos pela plataforma .NET (ie IReadOnlyCollection <T>)
Kevin Avignon
4

Se o objetivo é realmente evitar várias enumerações, é melhor ler a resposta de Marc Gravell, mas mantendo a mesma semântica, você pode simplesmente remover o redundante Anye as Firstchamadas e seguir com:

public List<object> Foo(IEnumerable<object> objects)
{
    if (objects == null)
        throw new ArgumentNullException("objects");

    var first = objects.FirstOrDefault();

    if (first == null)
        throw new ArgumentException(
            "Empty enumerable not supported.", 
            "objects");

    var list = DoSomeThing(first);  

    var secondList = DoSomeThingElse(objects);

    list.AddRange(secondList);

    return list;
}

Observe que isso pressupõe que você IEnumerablenão é genérico ou, pelo menos, está restrito a ser um tipo de referência.

João Angelo
fonte
2
objectsainda está sendo passado para DoSomethingElse, que está retornando uma lista, portanto a possibilidade de você ainda enumerar duas vezes ainda está lá. De qualquer maneira, se a coleção era uma consulta LINQ to SQL, você acessava o banco de dados duas vezes.
Paul Stovell
11
@PaulStovell, leia isto: "Se o objectivo é realmente evitar que várias enumerações que a resposta por Marc Gravell é o único a ler" =)
gdoron está apoiando Monica
Foi sugerido em algumas outras postagens de stackoverflow sobre o lançamento de exceções para listas vazias e vou sugerir aqui: por que lançar uma exceção em uma lista vazia? Pegue uma lista vazia, dê uma lista vazia. Como paradigma, isso é bem direto. Se o chamador não souber que está passando uma lista vazia, há um problema.
Keith Hoffman
@Kithith Hoffman, o código de exemplo mantém o mesmo comportamento do código que o OP postou, em que o uso de Firstlançará uma exceção para uma lista vazia. Eu não acho possível dizer nunca lançar exceção na lista vazia ou sempre lançar exceção na lista vazia. Depende ...
João Angelo
Justo João. Mais comida para reflexão do que uma crítica ao seu post.
21812 Keith Hoffman
3

Normalmente sobrecarrego meu método com IEnumerable e IList nessa situação.

public static IEnumerable<T> Method<T>( this IList<T> source ){... }

public static IEnumerable<T> Method<T>( this IEnumerable<T> source )
{
    /*input checks on source parameter here*/
    return Method( source.ToList() );
}

Tomo o cuidado de explicar nos comentários resumidos dos métodos que a chamada IEnumerable executará um .ToList ().

O programador pode optar por .ToList () em um nível superior se várias operações estiverem sendo concatenadas e depois chamar a sobrecarga IList ou deixar minha sobrecarga IEnumerable cuidar disso.

Mauro Sampietro
fonte
20
Isso é horrível.
Mike Chamberlain
11
@ MikeChamberlain Sim, é realmente horrível e totalmente limpo ao mesmo tempo. Infelizmente, alguns cenários pesados ​​precisam dessa abordagem.
Mauro Sampietro 11/10
11
Mas você recriaria a matriz toda vez que a passasse para o método parameratizado IEnumerable. Concordo com @ MikeChamberlain a esse respeito, esta é uma sugestão terrível.
Krythic
2
@ Krythic Se você não precisar recriar a lista, execute uma ToList em outro lugar e use a sobrecarga IList. Esse é o objetivo desta solução.
Mauro Sampietro 22/11
0

Se você precisar apenas verificar o primeiro elemento, poderá espiá-lo sem iterar toda a coleção:

public List<object> Foo(IEnumerable<object> objects)
{
    object firstObject;
    if (objects == null || !TryPeek(ref objects, out firstObject))
        throw new ArgumentException();

    var list = DoSomeThing(firstObject);
    var secondList = DoSomeThingElse(objects);
    list.AddRange(secondList);

    return list;
}

public static bool TryPeek<T>(ref IEnumerable<T> source, out T first)
{
    if (source == null)
        throw new ArgumentNullException(nameof(source));

    IEnumerator<T> enumerator = source.GetEnumerator();
    if (!enumerator.MoveNext())
    {
        first = default(T);
        source = Enumerable.Empty<T>();
        return false;
    }

    first = enumerator.Current;
    T firstElement = first;
    source = Iterate();
    return true;

    IEnumerable<T> Iterate()
    {
        yield return firstElement;
        using (enumerator)
        {
            while (enumerator.MoveNext())
            {
                yield return enumerator.Current;
            }
        }
    }
}
Madruel
fonte
-3

Primeiro, esse aviso nem sempre significa muito. Normalmente, eu o desabilito depois de me certificar de que não é um gargalo de desempenho. Significa apenas que a IEnumerableavaliação é feita duas vezes, o que geralmente não é um problema, a menos que o processo evaluationdemore muito. Mesmo que demore muito, nesse caso, você está usando apenas um elemento na primeira vez.

Nesse cenário, você também pode explorar ainda mais os poderosos métodos de extensão linq.

var firstObject = objects.First();
return DoSomeThing(firstObject).Concat(DoSomeThingElse(objects).ToList();

É possível avaliar apenas IEnumerableuma vez neste caso com alguma trabalheira, mas perfil primeiro e ver se é realmente um problema.

vidstige
fonte
15
Eu trabalho muito com o IQueryable e desativar esse tipo de aviso é muito perigoso.
gdoron está apoiando Monica
5
Avaliar duas vezes é uma coisa ruim nos meus livros. Se o método usar IEnumerable, talvez eu tente passar uma consulta LINQ to SQL, o que resulta em várias chamadas de banco de dados. Desde a assinatura do método não indica que ele pode enumerar duas vezes, ele deve ou alterar a assinatura (aceitar IList / ICollection) ou apenas iterate uma vez
Paul Stovell
4
otimizar antecipadamente e arruinar a legibilidade e, portanto, a possibilidade de fazer otimizações que diferem posteriormente é muito pior em meu livro.
vidstige
Quando você tem uma consulta ao banco de dados, garantir que ela seja avaliada apenas uma vez faz diferença. Não é apenas um benefício teórico futuro, é um benefício real mensurável no momento. Além disso, avaliar apenas uma vez não é apenas benéfico para o desempenho, mas também é necessário para a correção. A execução de uma consulta ao banco de dados duas vezes pode produzir resultados diferentes, pois alguém pode ter emitido um comando de atualização entre as duas avaliações da consulta.
11
@ DVD Eu não recomendo tal coisa. É claro que você não deve enumerar os IEnumerablesresultados diferentes a cada iteração ou que levam muito tempo para iterar. Veja a resposta de Marc Gravells - Ele também declara em primeiro lugar "talvez não se preocupe". O problema é que: muitas vezes você vê isso e não tem com o que se preocupar.
vidstige