Menangani peringatan untuk kemungkinan penghitungan berulang IEnumerable

359

Dalam kode saya perlu menggunakan IEnumerable<>beberapa kali sehingga mendapatkan kesalahan Resharper dari "Kemungkinan beberapa enumerasi IEnumerable".

Kode sampel:

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;
}
  • Saya dapat mengubah objectsparameter menjadi Listdan kemudian menghindari kemungkinan beberapa enumerasi tetapi kemudian saya tidak mendapatkan objek tertinggi yang bisa saya tangani.
  • Hal lain yang dapat saya lakukan adalah mengonversikan IEnumerableke Listdi awal metode:

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

Tapi ini hanya canggung .

Apa yang akan Anda lakukan dalam skenario ini?

gdoron mendukung Monica
sumber

Jawaban:

471

Masalah dengan mengambil IEnumerablesebagai parameter adalah bahwa ia memberi tahu penelepon "Saya ingin menyebutkan ini". Itu tidak memberitahu mereka berapa kali Anda ingin menghitung.

Saya bisa mengubah parameter objek menjadi Daftar dan kemudian menghindari kemungkinan beberapa enumerasi tapi kemudian saya tidak mendapatkan objek tertinggi yang bisa saya tangani .

Tujuan mengambil objek tertinggi adalah mulia, tetapi meninggalkan ruang untuk terlalu banyak asumsi. Apakah Anda benar-benar ingin seseorang untuk meneruskan permintaan LINQ ke SQL ke metode ini, hanya untuk Anda menghitungnya dua kali (mendapatkan hasil yang berpotensi berbeda setiap kali?)

Semantik yang hilang di sini adalah bahwa seorang penelepon, yang mungkin tidak meluangkan waktu untuk membaca rincian metode ini, mungkin menganggap Anda hanya mengulangi sekali - sehingga mereka memberi Anda objek mahal. Tanda tangan metode Anda tidak menunjukkan apa pun.

Dengan mengubah tanda tangan metode ke IList/ ICollection, Anda setidaknya akan membuatnya lebih jelas kepada penelepon apa harapan Anda, dan mereka dapat menghindari kesalahan mahal.

Jika tidak, sebagian besar pengembang yang melihat metode ini mungkin menganggap Anda hanya mengulanginya sekali saja. Jika meminum IEnumerablesangat penting, Anda harus mempertimbangkan untuk melakukan .ToList()di awal metode.

Sayang sekali. NET tidak memiliki antarmuka IEnumerable + Count + Indexer, tanpa metode Add / Remove etc., yang saya duga akan menyelesaikan masalah ini.

Paul Stovell
sumber
32
Apakah ReadOnlyCollection <T> memenuhi persyaratan antarmuka yang Anda inginkan? msdn.microsoft.com/en-us/library/ms132474.aspx
Dan Is Fiddling By Firelight
74
@DanNeely saya akan menyarankan IReadOnlyCollection(T)(baru dengan .net 4.5) sebagai antarmuka terbaik untuk menyampaikan gagasan bahwa itu adalah IEnumerable(T)yang dimaksudkan untuk disebutkan berulang kali. Seperti yang dinyatakan oleh jawaban ini, IEnumerable(T)itu sendiri sangat umum sehingga bahkan merujuk pada konten yang tidak dapat disetel ulang yang tidak dapat dihitung ulang tanpa efek samping. Tetapi IReadOnlyCollection(T)menyiratkan re-iterability.
binki
1
Jika Anda melakukan .ToList()di awal metode, Anda harus terlebih dahulu memeriksa apakah parameternya tidak nol. Itu berarti Anda akan mendapatkan dua tempat di mana Anda melempar ArgumentException: jika parameternya nol dan jika tidak memiliki item apa pun.
comecme
Saya membaca artikel ini tentang semantik IReadOnlyCollection<T>vs IEnumerable<T>dan kemudian memposting pertanyaan tentang penggunaan IReadOnlyCollection<T>sebagai parameter, dan dalam hal ini. Saya pikir kesimpulan saya akhirnya sampai pada logika untuk diikuti. Itu harus digunakan di mana pencacahan diharapkan, tidak harus di mana mungkin ada pencacahan ganda.
Neo
32

Jika data Anda selalu dapat diulang, mungkin jangan khawatir tentang hal itu. Namun, Anda juga dapat membuka gulungannya - ini sangat berguna jika data yang masuk bisa besar (misalnya, membaca dari disk / jaringan):

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;
}

Catatan Saya mengubah semantik DoSomethingElse sedikit, tetapi ini terutama untuk menunjukkan penggunaan yang belum dibuka. Anda bisa membungkus kembali iterator, misalnya. Anda bisa membuatnya menjadi blok iterator juga, yang bisa jadi bagus; maka tidak ada list- dan Anda akan yield returnitem yang Anda dapatkan, daripada menambah daftar untuk dikembalikan.

Marc Gravell
sumber
4
+1 Yah itu adalah kode yang sangat bagus, tapi- Saya kehilangan kecantikan dan keterbacaan kode.
gdoron mendukung Monica
5
@ Gdoron tidak semuanya cantik; p Saya tidak yang di atas tidak dapat dibaca, meskipun. Terutama jika itu dibuat menjadi blok iterator - cukup imut, IMO.
Marc Gravell
Saya bisa menulis: "var objectList = objects.ToList ();" dan simpan semua penanganan Enumerator.
gdoron mendukung Monica
10
@ gdoron dalam pertanyaan yang secara eksplisit Anda katakan tidak benar-benar ingin melakukan itu; masih diulang). Pada akhirnya, saya hanya menyajikan opsi. Hanya Anda yang tahu konteks lengkap untuk melihat apakah itu dibenarkan. Jika data Anda berulang, opsi lain yang valid adalah: jangan ubah apa pun! Abaikan R # sebagai gantinya ... itu semua tergantung pada konteksnya.
Marc Gravell
1
Saya pikir solusi Marc cukup bagus. 1. Sangat jelas bagaimana 'daftar' diinisialisasi, sangat jelas bagaimana daftar diulang, dan sangat jelas anggota mana dari daftar dioperasikan.
Keith Hoffman
9

Menggunakan IReadOnlyCollection<T>atau IReadOnlyList<T>dalam metode tanda tangan alih-alih IEnumerable<T>, memiliki keuntungan membuat eksplisit bahwa Anda mungkin perlu memeriksa penghitungan sebelum mengulangi, atau mengulangi beberapa kali karena alasan lain.

Namun mereka memiliki kelemahan besar yang akan menyebabkan masalah jika Anda mencoba untuk memperbaiki kode Anda untuk menggunakan antarmuka, misalnya untuk membuatnya lebih dapat diuji dan ramah terhadap proxy dinamis. Poin kuncinya adalah yang IList<T>tidak diwariskan dariIReadOnlyList<T> , dan juga untuk koleksi lain dan masing-masing antarmuka read-only. (Singkatnya, ini karena .NET 4.5 ingin menjaga kompatibilitas ABI dengan versi sebelumnya. Tetapi mereka bahkan tidak mengambil kesempatan untuk mengubahnya di .NET core. )

Ini berarti bahwa jika Anda mendapatkan IList<T>dari beberapa bagian dari program dan ingin meneruskannya ke bagian lain yang mengharapkan IReadOnlyList<T>, Anda tidak bisa! Namun Anda dapat lulus IList<T>sebagaiIEnumerable<T> .

Pada akhirnya, IEnumerable<T>adalah satu-satunya antarmuka read-only yang didukung oleh semua koleksi .NET termasuk semua antarmuka koleksi. Alternatif lain akan kembali menggigit Anda ketika Anda menyadari bahwa Anda mengunci diri dari beberapa pilihan arsitektur. Jadi saya pikir ini adalah tipe yang tepat untuk digunakan dalam tanda tangan fungsi untuk menyatakan bahwa Anda hanya ingin koleksi read-only.

(Perhatikan bahwa Anda selalu dapat menulis IReadOnlyList<T> ToReadOnly<T>(this IList<T> list)metode ekstensi yang dicetak sederhana jika tipe yang mendasarinya mendukung kedua antarmuka, tetapi Anda harus menambahkannya secara manual di mana-mana ketika melakukan refactoring, di mana seperti IEnumerable<T>yang selalu kompatibel.)

Seperti biasa ini tidak mutlak, jika Anda menulis kode database-berat di mana penghitungan berulang secara tidak disengaja akan menjadi bencana, Anda mungkin lebih suka trade-off yang berbeda.

Gabriel Morin
sumber
2
+1 Untuk datang pada posting lama dan menambahkan dokumentasi tentang cara-cara baru untuk mengatasi masalah ini dengan fitur-fitur baru yang disediakan oleh platform .NET (yaitu IReadOnlyCollection <T>)
Kevin Avignon
4

Jika tujuannya adalah benar-benar untuk mencegah beberapa enumerasi daripada jawaban oleh Marc Gravell adalah yang dibaca, tetapi mempertahankan semantik yang sama Anda dapat dengan mudah menghapus redundan Anydan Firstpanggilan dan pergi dengan:

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;
}

Perhatikan, bahwa ini mengasumsikan bahwa Anda IEnumerabletidak generik atau setidaknya dibatasi menjadi tipe referensi.

João Angelo
sumber
2
objectsmasih diteruskan ke DoSomethingElse yang mengembalikan daftar, sehingga kemungkinan Anda masih menghitung dua kali masih ada. Either way jika koleksi adalah permintaan LINQ untuk SQL, Anda menekan DB dua kali.
Paul Stovell
1
@PaulStovell, Baca ini: "Jika tujuannya benar-benar untuk mencegah beberapa enumerasi daripada jawaban oleh Marc Gravell adalah yang dibaca" =)
gdoron mendukung Monica
Telah disarankan pada beberapa posting stackoverflow lain tentang melempar pengecualian untuk daftar kosong dan saya akan menyarankannya di sini: mengapa melemparkan pengecualian pada daftar kosong? Dapatkan daftar kosong, berikan daftar kosong. Sebagai sebuah paradigma, itu cukup mudah. Jika penelepon tidak tahu mereka memasukkan daftar kosong, itu ada masalah.
Keith Hoffman
@Keith Hoffman, kode sampel mempertahankan perilaku yang sama dengan kode yang diposting OP, di mana penggunaan Firstakan melempar pengecualian untuk daftar kosong. Saya tidak berpikir mungkin untuk mengatakan tidak pernah membuang pengecualian pada daftar kosong atau selalu membuang pengecualian pada daftar kosong. Tergantung ...
João Angelo
Cukup adil Joao. Lebih banyak makanan untuk dipikirkan daripada kritik terhadap posting Anda.
Keith Hoffman
3

Saya biasanya membebani metode saya dengan IEnumerable dan IList dalam situasi ini.

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() );
}

Saya berhati-hati untuk menjelaskan dalam ringkasan komentar tentang metode yang memanggil IEnumerable akan melakukan .ToList ().

Pemrogram dapat memilih untuk .ToList () pada tingkat yang lebih tinggi jika beberapa operasi sedang digabungkan dan kemudian memanggil kelebihan IList atau membiarkan kelebihan IEnumerable saya yang mengurusnya.

Mauro Sampietro
sumber
20
Ini mengerikan.
Mike Chamberlain
1
@ MikeChamberlain Ya, itu benar-benar mengerikan dan benar-benar bersih pada saat yang sama. Sayangnya beberapa skenario berat membutuhkan pendekatan ini.
Mauro Sampietro
1
Tapi kemudian Anda akan menciptakan ulang array setiap kali Anda meneruskannya ke metode parameratized IEnumerable. Saya setuju dengan @MikeChamberlain dalam hal ini, ini adalah saran yang mengerikan.
Krythic
2
@Krythic Jika Anda tidak membutuhkan daftar untuk dibuat kembali, Anda melakukan ToList di tempat lain dan menggunakan kelebihan IList. Itulah inti dari solusi ini.
Mauro Sampietro
0

Jika Anda hanya perlu memeriksa elemen pertama, Anda dapat mengintipnya tanpa mengulangi seluruh koleksi:

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
sumber
-3

Pertama, peringatan itu tidak selalu berarti banyak. Saya biasanya menonaktifkannya setelah memastikan itu bukan leher botol kinerja. Itu hanya berarti IEnumerabledievaluasi dua kali, yang biasanya tidak menjadi masalah kecuali jika evaluationitu sendiri membutuhkan waktu lama. Bahkan jika itu memakan waktu lama, dalam hal ini Anda hanya menggunakan satu elemen pertama kali.

Dalam skenario ini Anda juga dapat mengeksploitasi metode ekstensi LINQ yang kuat bahkan lebih.

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

Dimungkinkan untuk hanya mengevaluasi IEnumerablesekali dalam kasus ini dengan beberapa kerumitan, tetapi profil terlebih dahulu dan lihat apakah itu benar-benar masalah.

vidstige
sumber
15
Saya banyak bekerja dengan IQueryable dan mematikan peringatan semacam itu sangat berbahaya.
gdoron mendukung Monica
5
Mengevaluasi dua kali adalah hal buruk dalam buku saya. Jika metode ini menggunakan IEnumerable, saya mungkin tergoda untuk mengirimkan query LINQ ke SQL, yang menghasilkan beberapa panggilan DB. Karena tanda tangan metode tidak menunjukkan bahwa ia dapat menghitung dua kali, ia harus mengubah tanda tangan (menerima IList / ICollection) atau hanya mengulangi sekali
Paul Stovell
4
mengoptimalkan awal dan merusak keterbacaan dan dengan demikian kemungkinan untuk melakukan optimasi yang membuat perbedaan nanti jauh lebih buruk dalam buku saya.
vidstige
Ketika Anda punya query database, memastikan itu hanya dievaluasi sekali sudah membuat perbedaan. Ini bukan hanya manfaat teoretis di masa depan, ini adalah manfaat nyata yang terukur sekarang. Tidak hanya itu, mengevaluasi hanya sekali tidak hanya bermanfaat untuk kinerja, itu juga diperlukan untuk kebenaran. Menjalankan kueri basis data dua kali dapat menghasilkan hasil yang berbeda, karena seseorang mungkin telah mengeluarkan perintah pembaruan antara dua evaluasi kueri.
1
@ DVD saya tidak merekomendasikan hal seperti itu. Tentu saja Anda tidak boleh menghitung IEnumerablesyang memberikan hasil yang berbeda setiap kali Anda mengulanginya atau yang membutuhkan waktu yang lama untuk beralih. Lihat jawaban Marc Gravells - Dia juga menyatakan pertama dan terutama "mungkin jangan khawatir". Masalahnya adalah - Banyak kali Anda melihat ini, Anda tidak perlu khawatir.
vidstige